Fix ticking behavior for child BlockEntity classes #194
Conversation
UpcraftLP
commented
Dec 31, 2024
UpcraftLP
commented
- fixes ticking component behavior for child classes if one of their superclasses has a ticking component but the child class does not
- add test case to verify the behavior
| if(StaticBlockComponentPlugin.INSTANCE.clientTicking.contains(superclass)) StaticBlockComponentPlugin.INSTANCE.clientTicking.add(entityClass); | ||
| if(StaticBlockComponentPlugin.INSTANCE.serverTicking.contains(superclass)) StaticBlockComponentPlugin.INSTANCE.serverTicking.add(entityClass); |
There was a problem hiding this comment.
Are we sure all parent classes went through this method before this class does ?
There was a problem hiding this comment.
I think so. If you can find a scenario where that isnt the case please let me know.
Anyway I added another test case for it.
There was a problem hiding this comment.
oh duh, there is the recursive call right above... But, consider a three-levels hierarchy A -> B -> C :
- A has ticking components
- B has non-ticking components
- C has no components for itself
when C loads, it will check only B, not A right ?
There was a problem hiding this comment.
wait nevermind, B will be considered ticking because the component building method is recursive, so no issue there.
| // if parent class needs to tick, this one does, too! | ||
| if(StaticBlockComponentPlugin.INSTANCE.clientTicking.contains(superclass)) StaticBlockComponentPlugin.INSTANCE.clientTicking.add(entityClass); | ||
| if(StaticBlockComponentPlugin.INSTANCE.serverTicking.contains(superclass)) StaticBlockComponentPlugin.INSTANCE.serverTicking.add(entityClass); |
There was a problem hiding this comment.
Also, could you maybe move this to a method in StaticBlockComponentPlugin instead of directly mutating its visible-for-testing "private" fields?
There was a problem hiding this comment.
I did not change it because VisibleForTesting's semantics say it would otherwise be assumed to be package-private, which would still allow calling it like this just fine.
But sure, will do
