ImmersiveMC

ImmersiveMC

2M Downloads

Make ActiveConfig a Proper Object

hammy275 opened this issue ยท 4 comments

commented

ActiveConfig is currently just a bunch of static fields and methods. I think it'd make more sense to have it be a fully fledged object, so that way code doesn't need to be duplicated between the S-->C config sync and the C-->S config sync. Plus, then we don't need a separate ServerPlayerConfig.

If #307 is done, we may want to merge the two into one object, containing the file config value and the active config value. Additionally, fields can be marked as whether or not they sync to the server during the C-->S config sync, so we don't send the full packet along the other direction.

commented

Something I may want to do with this is to construct the encode and decode functions by getting all the fields of the object, then writing and reading those en-mass. I'd likely want to sort the array containing all the fields just in case they're returned in a different order on different JVMs, unless it's something that's guaranteed.

Of course, this comes at the downside of reflection in the first place, but I think it's not much of an impact due to how rarely the packets are sent/received.

commented

Original refactor done in 1.20.4 at d28d3c2.

commented

Just about done moving the refactor to other branches, but caught a bug where, if you enter a singleplayer world with an immersive off, then enable it, it won't actually be enabled until you relog. Need to re-do that hack for singleplayer worlds to reload the config server-side if it's changed client-side.

Also, I see a loadActive() call to the config in ImmersivesCustomizeScreen.java, when it should be an ActiveConfig.FILE.loadFromFile().

Leaving this issue open until I fix the above.

commented

Bugs fixed!