Sponge icon indicating copy to clipboard operation
Sponge copied to clipboard

Post ChangeDataHolderEvent.ValueChange for GameMode changes

Open ItsDoot opened this issue 2 years ago • 1 comments

This may be possible without an Overwrite, but as far as I'm aware currently it would require more than 1 injection, and having to track state across injections can get hairy.

ItsDoot avatar Nov 24 '21 00:11 ItsDoot

I'm a little concerned about this in regards to offering data. If we were to use ServerPlayer#offer(Keys.GAME_MODE, GameModes.CREATIVE), for example, and a plugin changed it to adventure or cancelled the change, the following line wouldn't even bat an eyelid and (currently) call anything a success:

https://github.com/SpongePowered/Sponge/blob/f69698d16498c6c82958955039389278689e32ac/src/main/java/org/spongepowered/common/data/provider/entity/ServerPlayerData.java#L61

Now, this doesn't totally invalidate what you've done here, but we have to be careful that this isn't the whole story. In fact, just using setAnd won't help so much, because it's less black and white than "it worked" or "it didn't work". Indeed, we'd want to use setAndGetResult, but I also admit that my data knowledge is a little rusty and I'd need to work through it.

It's a bit of an oddball but it's something that it would be good to get a thought or two about before we pull this - thankfully this isn't a breaking change so we have some time to do so.

dualspiral avatar Dec 19 '21 20:12 dualspiral