czyzby / gdx-lml

:octocat: LibGDX utility libraries.
Apache License 2.0
159 stars 25 forks source link

XML-friendly macros #8

Closed jpsgamboa closed 8 years ago

jpsgamboa commented 8 years ago

I understand that macros might be too complicated to make XML friendly, but given the fact that it's so useful to separate template parts into multiple files, an <:import or <:template tag might be much common than other macros.

I wonder if it's possible to include an attribute to appease the IDE. Currently I'm doing, for example: <:template "views/menubar.lml" />

Which could become cleaner doing: <:template path="views/menubar.lml" /> or <:import template="views/menubar.lml" />

Is it possible?

czyzby commented 8 years ago

Yes. I implemented optional named parameters a while back, so this feature can be added without breaking the existing templates - if you pass a simple string to the tag, it will work exactly the same as before.

Now that I think of it, most macros can be refactored this way, including something problematic like <:if {argument} == 2> - this would still be readable: <:if condition="{argument}==2">.

jpsgamboa commented 8 years ago

I'm sorry it wasn't clear or me. It can be implemented but it isn't possible yet, correct?

czyzby commented 8 years ago

I meant "yes, it's possible to implement".

I'm already on it. Not possible yet, unfortunately - some macros are hardcoded as no-named-param type.

If you don't use DTD, stuff like <:if {a} == {b}> is faster to write, and strict XML syntax gets in the way - this was the reasoning behind this.

czyzby commented 8 years ago

Obscure, meta-macros (like newTag or newAttribute, which turn out even more verbose than their pure Java equivalent) will not have named attributes. Others will.

Due to its complexity and the fact that you can create new macros with custom attributes, meta macro (:macro) will also work as before. While powerful and useful, it's safe to say it's an advanced feature - you can use DTD for view templates, while keep your macros template separate. Kind of like in this project: https://github.com/czyzby/gdx-lml/tree/master/examples/gdx-lml-vis-websocket/core/assets/views/macros

However, previously macros had no attribute content assist at all - now DTD files will generate their default attribute names as well.

jpsgamboa commented 8 years ago

Sounds great. And by the time a new user wants to dive into macros, he will already be familiar with the basic functionality. IMO, it's on the basics people need content assist the most!

czyzby commented 8 years ago

It should already work if you use the latest snapshot version. Just make sure to run Gradle task with --refresh-dependencies.

Answering your initial question: import macro now supports path attribute (which points to a LML template of your choosing) and optional replace attribute which allows to pass data to the imported template. See gdx-lml-tests project for more info on this macro.

Also, regenerate your DTD file. It should contain expected attributes for macros (added when possible to determine; some macros can have any attributes, like the iteration ones). See https://github.com/czyzby/gdx-lml/tree/new-macro-sign/lml/dtd

I checked out every modified macro with the gdx-lml-tests and gdx-lml-vis-tests projects, but I'd appreciate your testing. See if it works as expected in an actual project. See this commit to check out which macros where affected - if you don't agree with some attribute names, this is the time to make your suggestions.

jpsgamboa commented 8 years ago

Can confirm the path attribute is working in my project and looks great! The DTD file includes the new macro attributes as expected.

Unfortunately I can't yet have an informed opinion on the attribute names, but I see nothing that stands out negatively. I shall be working with LML in the next few days though.

czyzby commented 8 years ago

That's great. I'm closing this issue, as other macros (new tag, new attribute, new macro) are unlikely to be converted in such a way due to various reasons. If you notice any bugs concerning macro attributes, don't hesitate to comment here.

jpsgamboa commented 8 years ago

Sorry for not noticing this earlier, but the path attribute works fine when I run the application via gradle run task, but throws a:

Caused by: com.badlogic.gdx.utils.GdxRuntimeException: File not found: path="views\menubar.lml" (Internal)

exception when I run it via the main class. Removing the path= attribute it runs fine, so it doesn't seem to be an issue with my working dir. Any clues?

czyzby commented 8 years ago

Use / in file path and check if it works.

Actually, it seems like you're using outdated library version with your non-Gradle run. I checked import macro in LML test projects via main method, not the Gradle task, and it worked as expected (there shouldn't be any difference, really - it's still the same code). How exactly do you run the application? Have you tried running gradle clean task? Did you clean and regenerate IntelliJ/Eclipse project? Have you refreshed dependencies?

I checked the current import macro code. There's simply no way that the macro tries to pass the whole path="views\menubar.lml" instead of attribute value (views\menubar.lml) when it detects path attribute (or any named attribute, for that matter). Must be out-of-sync kind of problem. Maybe your project somehow switched to 1.5.1.9.2 instead of the snapshot version?

jpsgamboa commented 8 years ago

I'm actually using /. But get the same result using \. I suppose gdx swaps / to \ at some point.

But the error File not found: path="views\menubar.lml" (Internal) seems to suggest that path= is being passed as part of the file path.

czyzby commented 8 years ago

It does suggest that.

Look at that though: https://github.com/czyzby/gdx-lml/blob/new-macro-sign/lml/src/main/java/com/github/czyzby/lml/parser/impl/tag/macro/AbstractImportLmlMacroTag.java#L85-L93

I'm assuming named attributes work - if they didn't, every single application with attributes would break. Import macro currently throws exception if it has any named attributes (path="anything" is a named attribute, since it has = sign), but missing path.

There is simply no difference between running your application with Gradle or "pure Java". It's the same code, the same VM - the only thing that might vary is the working directory, but in this case, this seems to be LML issue. Which cannot happen. In theory. Yeah.

I suggest cleaning your project and refreshing dependencies. Hell, turn the IDE on and off, this might help. I reuploaded snapshot versions to Maven Central, hope it helps.

That's a funny issue though, I wonder how it actually happened.

jpsgamboa commented 8 years ago

It's working. I'm not sure what was causing this, but it was as if gradle run was using the 1.6.1.9.2-SNAPSHOT and running via the main class was using a previous version, and not caring about the attribute at all.

Anyway it was on my end. Sorry about that!

czyzby commented 8 years ago

Sure, not a problem. I figured it's that kind of issue. I'm not a fan of using snapshots for this exact reason - I'm going to release 1.6 soon, but I'd like some more feedback first. After all, there were many changes in this version, so I appreciate some actual users testing it out and posting issues with any problems, really. (Also, there was no LibGDX release in a pretty long time, so there's that.)

jpsgamboa commented 8 years ago

Yeah it was a lesson on my part. I'll keep working on this over the next days, so I might have some more issues or requests.

If you don't mind me piggybacking on this post, because this fits the template importing thematic: Let's say I'm designing a desktop-like application with a menu-bar, side-bars, and lot's of buttons. Because of this I want to separate my "main template" in multiple files for maintainability, and import them to the right place.

Now on the Java side, do I have to keep all of my "onclick logic" for the various buttons in the various templates on the "root" AbstractLmlView (let's call it MainView.class), or is it possible to separate them into relevant classes (say MenuBar.class, etc.)?

Since LML will look for @LmlAction("foo") in the MainView.class I passed in lmlParser.createView(), I'm assuming separating the code wont be possible. But I'm afraid of ending up with too much code inside MainView.class.

czyzby commented 8 years ago

That's not only possible, but also advised.

You probably use one of LmlParser#createView methods - as long as you pass them an object (or class) that implements ActionContainer interface, their methods will be scanned using reflection and available in LML views. (BTW, AbstractLmlView already implements this interface.) But keep in mind that ActionContainers can be registered before you parse any templates, which allows you have "global" actions. API usage:

        Lml.parser()
           // Will be created automatically using MyActionContainer no-arg constructor:
           .actions("myContainer", MyActionContainer.class)
           // You can also construct your ActionContainer manually:
           .actions("other", new OtherActionContainer())
           // A custom, non-reflection based action - can be used for performance critical input:
           .action("myAction", new ActorConsumer<String, TextButton>() {
            @Override
            public String consume(TextButton actor) {
                return actor.getName();
            })
            /* ...other parser settings... */
            .build();

If you're wondering about name collisions - first of all, if non-reflection based ActorConsumer is found for your action, it is always preferred over ActionContainer methods. If you proceed action name with actionContainerId. (the one you used during action registration), method from the selected ActionConrainer will be used. This also speeds up method look up, as only 1 container has to be analyzed, but don't worry about it too much. If you don't proceed the method with container ID, the first found method matching the passed name will be returned.

So, for example:

<!-- Will invoke ActorConsumer and set method result as the text: -->
<textButton>$myAction</textButton>
<!-- Will invoke MyActionContainer#myAction or any method annotated with
    @LmlAction("myAction"): -->
<label onClick="myContainer.myAction"/>

In your case, you could create an ActionContainer for each of your imported templates.

By the way - this is a snippet from a new wiki page I'm working on:

Prefer custom macros over imports

It's tempting to import an external LML template. For example, you might want to create something like this at some point:

header.lml

        <table>
          <!-- {content} is provided by the importer. -->
          <label>{content}</label>
          <textButton growX="true">@thisIsJustAnExample</textButton>
        </table>

actualView.lml

        <table>
          <:import path="header.lml" replace="content">@myTitle</:import>
          <!-- Rest of view -->
        </table>

You can pass the {content} attribute, so you're fine, right? Wrong. First of all, as soon as you move your header template, actualView.lml will break. In addition to that, if you want to add more arguments to the imported template, it will not be as easy to read. It can be done, but it's not that obvious either. (For the record: you can use another {argument} in header.lml and then use one of argument assignment macros before importing, like <:assign key="argument" value="something"/>. It should work. But I shouldn't confuse you with stuff like that.)

Instead, define a single file with your macros:

myMacros.lml

        <:macro header content vertical="false">
          <!-- {vertical} is an optional attribute that defaults to false -->
          <table oneColumn="{vertical}">
            <!-- {content} is provided by the importer. -->
            <label>{content}</label>
            <textButton growX="true">@thisIsJustAnExample</textButton>
          </table>
        </:macro>

actualView.lml

        <table>
          <!-- vertical attribute is optional - if you don't pass it, it will
               become "false". -->
          <:header vertical="true">@myTitle</:header>
          <!-- Rest of view -->
        </table>

As you can see above, {content} is still replaced by the text between tags (in this case, @myTitle), because content was used as second :macro attribute. Instead of importing an external file - and having to load it each time - we defined a new macro once and now we can use it everywhere. You don't even need to remember the replace="content" attribute, as :header macro automatically replaces the argument you pointed during its creation (in this case: {content}.

It's also very easy to define new custom attributes, even with some pre-defined default values. Hell, you could have a macro with 20 attributes and as long as most (or all) of them have a default value, and your LML templates using it might still be readable. Macros might need a few more lines of code to create, but they're still worth it.

In case you're wondering, custom macros defined in LML templates are "permanently" added to the LmlParser instance when they are parsed. You do not have to include the same macro again and again in every template - load one template with your global macros once and then you can use them in every other template that you parse later. See gdx-lml-vis-websocket example project for global macro file usage in action.

Dtd generator will detect your custom macros and generate DTD files with their tags and attributes, so you don't have to worry about content assist - just remember to actually parse the macros file before generating DTD.

To sum up, import macros are good for prototyping or when you need to load and include an external file (for example: a user-defined template or a file stored on a server). To build view abstractions that are easy to extend and customize, use macros.

(Note: I might introduce named parameters for :macro before releasing 1.6, I'll let you know if I do.)

czyzby commented 8 years ago

I had some spare time and decided to implement named parameters for the rest of the macros. There are some limitations, though. (Some attributes need to be in a certain order.) See MetaLmlMacroTag docs for more info.

Updated :macro example, now with named parameters:

        <:macro alias="header" replace="content" vertical="false">
          <!-- {vertical} is an optional attribute that defaults to false -->
          <table oneColumn="{vertical}">
            <!-- {content} is provided by the importer. -->
            <label>{content}</label>
            <textButton growX="true">@thisIsJustAnExample</textButton>
          </table>
        </:macro>

        <!-- Invocation: -->
        <:header vertical="true">Label content.</:header>
jpsgamboa commented 8 years ago

Wow great answer! ActionContainer exactly what I needed to know about. Thanks!

Glad to hear you introduced more named parameters. It's another step into making LML fully XML. And the parameter order a very small price to pay IMO. (And wasn't an order already necessary for meta macros anyway? Atleast for the aliasand replace?)

Regarding the wiki snipped you posted here, I would just suggest including a simple code example of how to "load" the macros.lml file, since it becomes easier to follow:

// Processing global LML macros, available in all views:
lmlParser.parseTemplate(Gdx.files.internal("views/macros/Global.lml"));

And then, for more: See gdx-lml-vis-websocket example project for global macro file usage in action.

btw: Do you think it would be beneficial if I told you about wiki parts that I feel may be lacking a some examples?

czyzby commented 8 years ago

Yes, the order was already forced. But generally named attributes bring optional parameter order, so meta-macro is currently the only exception from that rule.

Wiki is lacking in general. I'm going to remove the old syntax page, as it's redundant with the DTD around. Actually, I think about a rewrite once 1.6 is up. Any wiki suggestions are appreciated.

czyzby commented 8 years ago

Please, use #10 to discuss the wiki.

czyzby commented 8 years ago

@jpsgamboa, new libraries version is likely to be released soon. Although I was originally planning on waiting for LibGDX 1.9.3, it seems that it won't be released as quickly as I thought.

Anyway, have you experienced any bugs, awkward moments or missing features when using gdx-lml-vis with DTD support? I think that the whole LML DSL is XML-friendly right now, but I wouldn't want to miss anything.

jpsgamboa commented 8 years ago

It has been working nicely. I've experimented with a lot of widgets/actors and suggestions seem accurate. Haven't tried all of them though, so something might me missing, but I don't think there is reason for concern.

czyzby commented 8 years ago

Thanks. I'd appreciate further testing of the snapshots, just make sure to run Gradle with --refresh-dependencies every commit once in a while. I recently added support for new VisUI Spinner widget, it might be of some use. It seems that both VisUI and LML will be released around Monday.

czyzby commented 8 years ago

@jpsgamboa 1.6.1.9.2 is released. You can finally stop using snapshots. ; )