Fixed DebugRecursively attribute. #47
Conversation
|
|
||
| /** See {@link Group#setDebug(boolean, boolean)}. Second method argument is always true, so this method sets debug for | ||
| * all children; for non-recursive debug, use default debug attribute. Mapped to "debugRecursively". | ||
| * | ||
| * @author MJ */ | ||
| public class DebugRecursivelyLmlAttribute implements LmlAttribute<Group> { | ||
| private static class DebugAction implements ActorConsumer<Object, Object> { |
There was a problem hiding this comment.
You don't have to use <Object, Object> generics, I'm pretty sure you can just go ahead and use <Group, Void>, so you don't have to cast the actor to Group manually.
There was a problem hiding this comment.
Second generic parameter is used as an argument in consume. So I could use <Void, Group>, but LmlUserObject#addOnCloseAction is requiring ActorConsumer<?, Object> so I have to use <Void, Object> but I think <Object, Object> looks more clear.
There was a problem hiding this comment.
Yeah, I might have mixed it up with Java 8 Function. Now that I think about it, the generics order could have been better.
LmlUserObject is pretty much internal API, and I may have chosen some questionable designs to escape Java's generics and reflection hell. I'll accept this PR as is (once I have some time to test it - I'm currently at work), and I'll see if this can be achieved without ActorConsumer altogether.
There was a problem hiding this comment.
I have tried to make it similar to OnClose attribute, as it is also processed after the actor's tag is closed.
|
Your implementation was OK (and it seems that I used some similar "tricks" to set up similar properties), but I usually went with anonymous classes for readability. (Although they are far harder to read than Java 8 lambdas, but hey - we all love backwards compatibility.) Even if they have some slight overhead on mobile platforms, in case of debug rendering attribute I don't think we should worry too much about an extra class+object being defined. I refactored the code slightly to match other similar attributes. Still, thanks for finding the bug and actually fixing it instead of just creating an issue. ; ) |
DebugRecursively attribute needs to be processed after all children are added.