Skip to content

Fix BUTTON_TARGET skin property (pacemaker target setting display during music select)#740

Open
wcko87 wants to merge 1 commit intoexch-bms2:masterfrom
wcko87:fix_pacemaker_setting
Open

Fix BUTTON_TARGET skin property (pacemaker target setting display during music select)#740
wcko87 wants to merge 1 commit intoexch-bms2:masterfrom
wcko87:fix_pacemaker_setting

Conversation

@wcko87
Copy link
Copy Markdown
Contributor

@wcko87 wcko87 commented May 26, 2023

Issue

Currently, the BUTTON_TARGET = 77 skin property is not working, because it has been removed from IntegerPropertyFactory.

This means that for many skins, we cannot see the pacemaker (target) option during music select.

Deprecating BUTTON_TARGET may have been intentional as targetlist can now be modified (04d412a), so the string value of the target is preferred over the index. However this breaks the functionality of many old skins.

Changes

I have changed PlayerConfig.java to internally store the index of the pacemaker (target) option instead of the string. However, the get and set methods use the string value, so string functionality is maintained.

We store the index internally so that it can be retrieved from IntegerPropertyFactory.java without having to do a loop each time the value is retrieved.

Also, the loop in EventFactory.java has been removed because now the index can be retrieved directly.

Note 1

Old skins will still be unable to display all pacemaker options (e.g. IR options), but they at least they can still show the old options.

pacemaker_input.mp4

Maybe it will be good to rename the option to BUTTON_TARGET_DEPRECATED = 77, to maintain compatibility with old skins while encouraging skin makers to use the StringProperty instead.

Note 2

I removed the setTargetlist() function for targetlist in PlayerConfig.java because it is not used. If this is not preferable, please add it back.

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.

1 participant