Skip to content
This repository has been archived by the owner on May 27, 2021. It is now read-only.

[LML] Resolve LmlActor annotation value from field name #65

Open
metaphore opened this issue Dec 15, 2017 · 14 comments
Open

[LML] Resolve LmlActor annotation value from field name #65

metaphore opened this issue Dec 15, 2017 · 14 comments

Comments

@metaphore
Copy link
Contributor

metaphore commented Dec 15, 2017

I feel like I might be missing something, but still I don't see any reason against this approach.

If there is no value specified for LmlActor annotation, it could be resolved implicitly from related field name, e.g. @LmlActor Button button; = @LmlActor("button") Button button;.

This might be not universal since, value is array and annotation may be assigned to collection field type. But I see singular widget declaration as a general case and duplicating field name most of the time makes code cumbersome.

@czyzby
Copy link
Owner

czyzby commented Dec 15, 2017

There might have been some issues with this approach (valid LML IDs do not have to be valid Java identifiers and actors collections you mentioned), but I don't see why it shouldn't be added. Not sure if LibGDX reflection API allows to do that at this point, but I'll look into it when I finally force myself to update LML. ;) I'll try this weekend.

@metaphore
Copy link
Contributor Author

I believe so, since you already use com.badlogic.gdx.utils.reflect.Field in AbstractLmlParser, it should be simple like Field#getName().

Thanks a lot, really looking forward for this little but very useful improvement.

@czyzby
Copy link
Owner

czyzby commented Dec 15, 2017

Yeah, you need a reference to the Field to perform the actual injection, I don't expect this to be hard to implement.

@metaphore
Copy link
Contributor Author

BTW, I just thought the same technique could be applied to LMLAction annotation. ActionContainerWrapper#mapClassMethods(Class<?>) operates reflected com.badlogic.gdx.utils.reflect.Method and thus could easily extract method name.

@czyzby
Copy link
Owner

czyzby commented Dec 16, 2017

I think I originally wanted to avoid that partially because applications can be obfuscated with Proguard before the release, changing field and method names. This will either break your game or force you to exclude extra classes from the Proguard settings. Even though it might seem redundant, it's actually a sensible practice to keep meta-data in the annotations.

That said, it will certainly reduce boilerplate required to define LML view handlers, so I don't see why we shouldn't include that.

@metaphore
Copy link
Contributor Author

I've never played much with code obfuscation and can only guess, but should not those runtime methods like Field#getName() return actual post obfuscated names?

@czyzby
Copy link
Owner

czyzby commented Dec 16, 2017

Unless you specifically tell the obfuscation lib not to rename fields/methods, getName will still work, but will return meaningless obfuscated names like aaA, which obviously won't match your LML IDs. Obfuscation will not modify IDs passed to annotations, though.

@metaphore
Copy link
Contributor Author

Ah, text references to fields and methods from LML templates, right. I think it's a common practice to note if library requires any extra Proguard rules, at least, as I could remember, a lot of top used Android libs describe it under corresponding readme/wiki section or supply within an artifact.

@czyzby
Copy link
Owner

czyzby commented Dec 17, 2017

1.9.1.9.8-SNAPSHOT is currently being uploaded. I think I've fixed all crucial issues that were open. @metaphore Can you use the latest version and test if the solution works for you? I'm planning on uploading the 1.9.1.9.8 release next week. Thanks.

@metaphore
Copy link
Contributor Author

Thanks, will do!
But what about VisUI, as of I know it is still not updated to LibGDX 1.9.8 and should do that sometime soon. Maybe it would be better to wait for it and release only then?

@czyzby
Copy link
Owner

czyzby commented Dec 18, 2017

Isn't VisUI development halted? There was no release for 1.9.7. For what it's worth, VisUI 1.4.0 should work with latest LibGDX release.

@metaphore
Copy link
Contributor Author

I can't say for sure if current VisUI is compatible with latest LibGDX, but @kotcrab is working on the new version kotcrab/vis-ui#275

@metaphore
Copy link
Contributor Author

@czyzby I've had some time playing with 1.9.1.9.8-SNAPSHOT and it seems pretty stable. I thought it's ready to go, but at the last moment I decided to make PR and drop some of my local LML changes, so maybe you would find them useful and include in this release #67

@czyzby
Copy link
Owner

czyzby commented Dec 26, 2017

Thanks. I'll wait for the release until the latest VisUI version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants