Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added hotkey macro button properties to metamacro functions #4970

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

bubblobill
Copy link
Collaborator

@bubblobill bubblobill commented Oct 2, 2024

Identify the Bug or Feature request

resolves #4897

Description of the Change

included fields in metamacro functions for macro button properties "hotkey" and "displayHotkey"

Possible Drawbacks

conflicts in hotkey settings from different sources - buyer beware

Documentation Notes

Macro hotkeys and their display can now be set through metamacro functions.

Release Notes

Macro hotkeys and their display can now be set through metamacro functions.


This change is Reviewable

@cwisniew
Copy link
Member

cwisniew commented Oct 2, 2024

It should check to see if a hot key exists before setting it, any conflict should be resolved by not updating the hotkey. As a macro writer I don't know what hotkeys a user has set up and how dependant they are on said hotkeys so overwriting them could cause usability issues

@bubblobill
Copy link
Collaborator Author

I'm more a fan of buyer beware. Better to add a getMacroHotKeys() function and make the macro coder responsible.

@bubblobill
Copy link
Collaborator Author

Or stick hot keys in getInfo() if it isn't already present.

@bubblobill
Copy link
Collaborator Author

There you go. Now people can perform their own checks.

@FullBleed
Copy link

There you go. Now people can perform their own checks.

Glad you went with a new function.

@cwisniew
Copy link
Member

cwisniew commented Oct 2, 2024

I'm more a fan of buyer beware. Better to add a getMacroHotKeys() function and make the macro coder responsible.

This doesn't solve the problem, the hot keys user has set up should not be over ridden by macro. Just like macros should override people's themes.

@FullBleed
Copy link

I'm more a fan of buyer beware. Better to add a getMacroHotKeys() function and make the macro coder responsible.

This doesn't solve the problem, the hot keys user has set up should not be over ridden by macro. Just like macros should override people's themes.

I get what you're saying... but without getting into the whole argument of what GMs/Frameworks should be allowed to do when trying to impart a particular experience... why not provide user guardrails for features that could be problematic to them instead of stopping framework devs from doing stuff that many people would not care about?

In this case, allow the function, but add a check-box in Preferences>Accessibility to block macro changes to Hot Keys. Put it on by default. That way it will be on the game-runner to instruct players to disable the block... and up to the players to accept the request.

@cwisniew
Copy link
Member

cwisniew commented Oct 2, 2024

I'm more a fan of buyer beware. Better to add a getMacroHotKeys() function and make the macro coder responsible.

This doesn't solve the problem, the hot keys user has set up should not be over ridden by macro. Just like macros should override people's themes.

I get what you're saying... but without getting into the whole argument of what GMs/Frameworks should be allowed to do when trying to impart a particular experience... why not provide user guardrails for features that could be problematic to them instead of stopping framework devs from doing stuff that many people would not care about?

The guardrail is you can't override a hotkey that is already set :)
I guess we could add that option but, then which would framework developers prefer to happen by default none of their hot keys work or just the ones that are already taken? I can already tell you that if it gets put in settings people rarely know about it. I will start a discussion in discord and let people decide

The above function when a framework could check if its already set and not do it is not adequate because people developing frameworks wont check, then it gets reported as a bug then I would be inclined to fix it, then framework developers report it as a bug...

@bubblobill
Copy link
Collaborator Author

Fine then. I'll have it return false or something in the new props if it would have overridden an existing key. Party pooper.

@bubblobill
Copy link
Collaborator Author

bubblobill commented Oct 4, 2024

Right then. Now the only way to change an assigned hotkey via macro is presumably to delete the hotkey holder before giving it to yourself. Assuming deleting a macro removes its hot key from the master list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Add hotkey to macro props for createMacro
3 participants