Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate parent id in custom control #336

Open
lyrgard opened this issue May 20, 2015 · 2 comments
Open

Duplicate parent id in custom control #336

lyrgard opened this issue May 20, 2015 · 2 comments

Comments

@lyrgard
Copy link

lyrgard commented May 20, 2015

I have a custom ImageButton control defined as :

     <controlDefinition name="imageButton" style="nifty-panel-style" controller="fr.lyrgard.hexScape.gui.desktop.controller.DefaultImageButtonController">
        <panel style="#panel" childLayout="horizontal" focusable="true" valign="center" visibleToMouse="true" width="$width" heigth="$height">
            <image id="#imageButtonImage" filename="$image" valign="center" align="center">
            </image>
            <panel width="10px"/>
            <text text="$text" style="nifty-label" />
            <effect>
                <onHover name="nop" onStartEffect="onStartHover()" onEndEffect="onStopHover()" />
                <onHover name="changeMouseCursor" id="buttonHoverMousePointer" />
            </effect>
            <interact onClick="doOnClick()" />
        </panel>
    </controlDefinition>

that works as intended. When I use this control in a panel, and I give it an id (let's say "someButtonId"), the image inside this controle get "someButtonId#imageButtonImage" as id, as expected, and the DefaultImageButtonController is able to get the Image element with :
element.findElementByName(element.getId() + "imageButtonImage");
in its bind method.

Now I created a more complex custom control that use an imageButton control inside :

<controlDefinition name="armyCardPanel" style="nifty-panel-style" controller="fr.lyrgard.hexScape.gui.desktop.controller.ArmyCardPanelController">
        <panel style="#panel" childLayout="horizontal" focusable="true" valign="center" visibleToMouse="true" width="250px" heigth="100px">
            <panel>
            </panel>
            <panel width="50px" height="100px" childLayout="vertical">
                <image id="#armyCardImage" filename="$image" width="50px" height="50px"/>
                <text text="" style="nifty-label"/>
                <control name="imageButton" align="center" height="24" width="24" valign="center" visibleToMouse="true"  
                        id="#putOnMapButton" 
                        goTo="homeScreen"
                        image="gui/icons/addAllPieces.png" 
                        imageHover="gui/icons/addAllPiecesHover.png" 
                        imagePressed="gui/icons/addAllPieces.png" />
            </panel>
        </panel>
    </controlDefinition>

If I use this controle (with id="someArmyCardPanelId"), in the DefaultImageButtonController, element.getId() returns "someArmyCardPanelId#putOnMapButton", as expected. However, element.findElementByName(element.getId() + "imageButtonImage"); returns null. I checked the elements tree, and found that the image element inside the imageButton control got "someArmyCardPanelId#putOnMapButton#putOnMapButton#imageButtonImage" as id.

Why is "putOnMapButton" twice in this id ? I was expecting "someArmyCardPanelId#putOnMapButton#imageButtonImage"

@void256
Copy link
Member

void256 commented May 27, 2015

I think we've tried to find the reason for that duplicate id part some time ago but haven't been succesful iirc 😉

However you shouldn't rely on the internal format of the id at all and iirc you don't have to!

You can lookup any sub-element by its regular id only. Nifty should take care of the resolving internally, so you should be able to simply issue a element.findElementByName("imageButtonImage").

@lyrgard
Copy link
Author

lyrgard commented May 28, 2015

Ok
I searched a little the source of this bug, and I think I found it.

When a ElementType is created, the method prepare(nifty, screen, rootElementType) is called.
This method will call makeFlatControls(), that will call itself reccursivly on each child element
Only thing that method do apart calling itself reccursivly is calling makeFlatControlsInternal() at the end.
This method makeFlatControlsInternal do nothing in ElementType, and only in ControlType is overriden. For ControlType, it calls mergeFromElementType (dunno what it does exactly), then calls resolveIds(this, getAttributes().get("id"))

resolveIds take care of changing the #id to parentId#id

So, let's say I have :

  • a control B that contains an image with id "#idImage"
  • a control A with id "idA" that contains a control B with id "#idB"

What hapens is (in pseudo code) :

  • A is the parent, so A.makeFlatControls() will be called by its own parent.
  • A will then call B.makeFlatControls().
  • B will call that on its own children, then will call B.resolveIds(B, "#idB") (here B's id is still "#idB" because A.resolveIds(A, "idA") has not yet been called).
  • B.resolveIds(B, "#idB") will be called reccursively on B children and will change the image id to "#idB#idImage", then will end.
  • flow returns to A.makeFlatControls(), that will finally call A.resolveIds(A, "idA")
  • A.resolveIds(A, "idA") will be called reccursivly on its children, including B (and will change its id to the expected *"idA#idB"*)... and the image !
  • The id of the image will be changed to its parentId ("idA#idB") + its own id (now being "#idB#idImage"), resulting in "idA#idB#idB#idImage" !!

The effect will get worse and worse the more you nest controls.

To prevent that, I propose the following change :

  • In ElementType.makeFlatControls(), call makeFlatControlsInternal(); before the for loop, instead of after.
  • In ControlType.resolveIds(parent, grandParentId), in the for loop, do not call resolveIds(element, grantParentId); if element is of type ControlType

What will then happens :

  • A is the parent, so A.makeFlatControls() will be called by its own parent.
  • A call will call its own makeFlatControlsInternal(), that will then call A.resolveIds(A, "idA")
  • A.resolveIds(A, "idA") will change B's id to "idA#idB" but won't call itself on B because B is of type ControlType
  • A will then loop on its children and call makeFlatControls() on them. especially B.makeFlatControls()
  • B.makeFlatControls() will call B.resolveIds(B, "idA#idB") (because here, B's id already is "idA#idB")
  • B.resolveIds(B, "idA#idB") will make the image id to the expected "idA#idB#idImage"

This solution should stay correct whatever the number of nested controls.

This is a little contribution to thanks you for all the time you invested into nifty-gui. Thanks a lot !!

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

No branches or pull requests

3 participants