exch-bms2 / beatoraja

Cross-platform rhythm game based on Java and libGDX.
GNU General Public License v3.0
627 stars 146 forks source link

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

Open wcko87 opened 1 year ago

wcko87 commented 1 year ago

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.

https://github.com/exch-bms2/beatoraja/assets/27341392/8592c9f4-9c1e-4a0b-bead-448b1d832c08

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.