Fire ChangeWorldGameRuleEvent

[thibaulthenry]

Hey,

As @dualspiral advised me, I'm trying to develop by my own as much as I can instead of reporting issues.
So, I try to make Sponge fire the ChangeWorldGameRuleEvent which was not used.

There is two (maybe 3) places where we have to fire this event :

• When using the vanilla command /gamerule
• When using the API to modify game rule
• And maybe when using the API to remove game rule. I need your opinion to decide.

This is my first time using the mixins so this is why I marked this PR as WIP, if you have any remarks just let me know.

There is also another point. I would like to give the information for plugin makers that modification of a game rule is cancellable. It means that in the API, WorldProperties#setGameRule should return a boolean instead of void.
Is this possible or is this a breaking change ?

Thank you for reading this PR.

[SizableShrimp]

For normal usage, changing a function’s type from void to anything else will not break stuff although not sure how that works in the scope of Sponge. Also, make sure to import Sponge’s standardized checkstyle.xml and format your code with it instead since you added a star import accidentally.

[dualspiral]

For normal usage, changing a function’s type from void to anything else will not break stuff although not sure how that works in the scope of Sponge

That is absolutely wrong. It would require any plugins that build against the current API to be recompiled because it is still a signature change. In this case, any plugin that uses WorldProperties#setGameRule will break because they're expecting to invoke the method void setGameRule(String, String); - changing the return type means that method no longer exists because it is now boolean setGameRule(String, String);. While the change might be source compatible, it is not binary compatible - we need both. This holds true for "normal usage" as well as Sponge if an API is considered stable. The return type is part of the API.

TL:DR; do not change any existing methods in the API for API 7.

[thibaulthenry]

Is there any solution to tell that WorldProperties#setGameRule might not have been completed ? (when event cancelled)

[dualspiral]

No, at least, not one I'd consider acceptable. We actually had this issue when we were implementing the KickPlayerEvent - in the end the event wasn't cancellable so we didn't make it cancellable which side stepped the issue - I get that's not the case here. However, the next oppotunity to change it is API 8 - you may want to look at the API there to see if it's been updated.

[thibaulthenry]

Well I guess I have to take the train of API-8 before it's too late

[thibaulthenry]

As SpongeAPI-7 modification is not possible, I guess this PR is not WIP anymore.
Unless we decide to also fire the event on game rule removal ?

[SizableShrimp]

changing the return type means that method no longer exists because it is now boolean setGameRule(String, String);. While the change might be source compatible, it is not binary compatible - we need both. This holds true for "normal usage" as well as Sponge if an API is considered stable. The return type is part of the API.

TL:DR; do not change any existing methods in the API for API 7.

Totally my fault for not realizing this. I don't investigate compiled code often and didn't know that compiled code points to a method with the specific signature, although that makes a lot more sense and something I forgot to consider. Apologies for my dumb comment since I don't make my own APIs often.

[dualspiral]

I'm going to close this for the same reasons as #3226 (comment) - but thanks as it did influence API 8.