search

attickdoor / XIVComboPlugin

Plugin version of the icon replacement features in dalamud
GNU General Public License v3.0
178 stars 282 forks source link

CustomComboPreset is running out of bits #313

Open skycoop opened 1 month ago

skycoop commented 1 month ago

While fiddling with #309, I noticed that the CustomComboPreset enum is pretty much out of bits to work with. After poking around a bit, I think there's two options

  1. Use a custom class that matches the API of the Flags enum. I found a library called InfiniteEnumFlags but it seems a little overcomplicated for this use case and doesn't seem to be directly serializable.
  2. Split the job configurations into their own enums (probably using short instead of int or long)

I think either approach will require a breaking change to the config, so I lean towards 2 being the better option since it would scale better long term. Totally willing to work on a fix, but wanted to open a ticket first for discussion 🙂

attickdoor commented 1 month ago

Yeah, I was just about to comment about this. The latter option is probably what I'll go with, and I think it'll have to be destructive. There's no good way to migrate from an unknown previous version to a job-based config. Will think about this in the future.

skycoop commented 1 month ago

I think it might be possible to convert from v4 to a new version by creating a mapping of the old enum to the new ones and converting the config on load and saving the new one, but it would definitely be pretty cumbersome.

Fenyn commented 1 month ago

I also noticed this while working on the SMN PR and mentioned it over there too. My initial thought was to split the configurations into job-specific enums that hold flags for each job individually, so I like the sound of option2 as well. :)

attickdoor commented 1 month ago

convert from v4

Big "yes, but" here. The config has been v4 for a while, and flags have shuffled in and out since then. So while a mapping could be made, it would be a bit iffy depending on when in time the user last updated the plugin.

skycoop commented 1 month ago

Ah, got it. I guess when a v4 config is loaded we need to reset it to a new version and warn the user with a popup? I'll see if I can knock this out today or tomorrow 🙂

skycoop commented 1 month ago

OK so I guess I bit off more than I could chew here. Breaking up the enums and updating uses in IconReplacer was straightforward enough, but I'm running into some issues with the configuration GUI. I'm not hugely familiar with C# so I'm running into issues figuring out how to build the GUI without resorting to hardcoding every job or ending up with awkward access patterns everywhere else 😕 I'll keep working on it, but just wanted to provide an update