search

u3d-community / U3D

Open-source, cross-platform 2D and 3D game engine built in C++
https://u3d.io
MIT License
160 stars 26 forks source link

Fix incorrectly read 'wrapmode' in Animatable #69

Open Lys0gen opened 1 month ago

Lys0gen commented 1 month ago

'wrapmode' attribute was incorrectly read from XMLElement instead of the attributeanimation, always defaulting it to WM_LOOP

SirNate0 commented 1 month ago

Is this incorrect? I don't really use the feature, so I have no preference either way. Just looking at the Platformer2D scene I saved a while back, it seems to put the wrapmode as an xml attribute on the attributeanimation element. E.g:


    <node id="888">
        <attribute name="Is Enabled" value="true" />
        <attribute name="Name" value="Background" />
        <attribute name="Tags" />
        <attribute name="Position" value="12.16 8.64 0" />
        <attribute name="Rotation" value="1 0 0 0" />
        <attribute name="Scale" value="3.5 3.5 3.5" />
        <attribute name="Variables" />
        <attributeanimation name="Rotation" interpolationmethod="Linear" wrapmode="Loop" speed="0.05">
            <keyframe time="0" type="Quaternion" value="1 0 0 0" />
            <keyframe time="1" type="Quaternion" value="-4.37114e-08 0 -0 1" />
            <keyframe time="2" type="Quaternion" value="1 0 0 0" />
        </attributeanimation>
        <component type="StaticSprite2D" id="1016">
            <attribute name="Layer" value="-99" />
            <attribute name="Sprite" value="Sprite2D;Textures/HeightMap.png" />
            <attribute name="Color" value="0.633564 0.504501 0.172582 1" />
            <attribute name="Draw Rectangle" value="-5.125 -5.125 5.125 5.125" />
            <attribute name="Texture Rectangle" value="4.87805e-08 1 1 4.87805e-08" />
        </component>
    </node>

If it is, it also needs to be changed here: https://github.com/u3d-community/U3D/blob/b661a8422f1eb4c4282d61d83c533e1cf3bb8def/Source/Urho3D/Scene/Animatable.cpp#L206

Lys0gen commented 1 month ago

You are exactly right, the 'wrapmode' is part of the 'attributeanimation'. In SaveXML it is already correct as far as I can see.

However the LoadXML was reading it from 'source', which is the parent XMLElement of attributeanimation 'elem', the parent obviously does not (and should not) have this attribute and therefore it always read and evaluated an empty string => default to WM_LOOP.

Or, to take your example, it would try to read the 'wrapmode' attribute from your < node id="888" > and not the <attributeanimation name="Rotation" ...