Looky1173
commented
11 years ago Imported from https://bugs.launchpad.net/or/+bug/1127500/comments/1
| Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Sat, 16 Feb 2013 20:44:29 GMT |
Closed Looky1173 closed 11 years ago
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Sat, 16 Feb 2013 20:44:29 GMT |
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Sat, 16 Feb 2013 20:44:59 GMT |
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | James Ross (twpol) |
| Date posted | Sun, 17 Feb 2013 12:46:50 GMT |
Fixed in V1448.
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Sun, 17 Feb 2013 14:40:41 GMT |
Thank you James, I saw you have chosen the other way. :-) But the other patch for Shapes.cs I attached needs also to be applied.
The reason is that there are cases, when an anim_node has only e.g. 5 keyframes. But we call PosableShape.AnimateMatrix() with all 8 frames. We set in line 217: var xnaPose = SharedShape.Matrices[iMatrix]; // start with the intial pose in the shape file
When key is e.g. 6, then this xnaPose is not get set to something else, it remains in initial pose state. That is a problem, because an animation with only 5 frames starts to animate back to its initial pose, but we want it to stay in pose 5!
That's why the patch I submitted for Shapes.cs is needed. You can see the wrong behavior at vehicle I linked in my first post in this bug thread. Start opening the door, and see the stair below that. The stair comes out, as expected, but then goes back too! It should go back only at door closing time. The patch corrects this problem.
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | James Ross (twpol) |
| Date posted | Mon, 18 Feb 2013 20:31:50 GMT |
This seems to be a more fundamental difference in how OR animates things; in Open Rails' current code, animations loop continuously, i.e. it transitions smoothly from the last key frame back around to the first. At least from the information I've found, it seems MSTS does not do this (which is why you need 9 frames for wheel/running gear animations with the last frame equal to the first, I believe).
I'm going to test out changing this in OR to not transition between the last two key frames.
Also, re: the MSTSWagon change, the use of Math.Max on the controllers seems to result in better behaviour - you don't get a couple of seconds pause before the doors close when you press the key, because the frame only has to count down from ~3 instead of 8 or whatever it is in the overall shape.
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Mon, 18 Feb 2013 21:07:34 GMT |
No, I think I haven't described the problem quite well. There is no need for any fundamental change of animation, I think, it is absolutely fine to transition between all key frames, including the last too. My problem lies elsewhere. Just look at the following .s file configuration, cut from the vehicle I linked above:
animations ( 1 animation ( 8 50 anim_nodes ( 21 anim_node MAIN ( controllers ( 0 ) ) anim_node DOOR_C ( controllers ( 1 linear_pos ( 4 linear_key ( 0 0.976703 0.42639 2.86327 ) linear_key ( 1 1.06052 0.42639 2.86327 ) linear_key ( 2 1.21618 0.42639 2.86327 ) linear_key ( 3 1.3 0.42639 2.86327 ) ) ) ) anim_node PantographTop2A ( controllers ( 1 linear_pos ( 2 linear_key ( 0 1.4257 2.74697 0.995314 ) linear_key ( 1 1.4357 2.74697 0.995314 ) ) ) ) anim_node DOOR_A ( controllers ( 1 linear_pos ( 8 linear_key ( 0 1.43556 1.81121 3.26894 ) linear_key ( 1 1.43556 1.81121 3.26894 ) linear_key ( 2 1.43556 1.81121 3.26894 ) linear_key ( 3 1.43556 1.81121 3.26894 ) linear_key ( 4 1.43556 1.81121 3.26894 ) linear_key ( 5 1.49355 1.81121 3.26894 ) linear_key ( 6 1.49355 1.81121 3.65282 ) linear_key ( 7 1.49355 1.81121 4.03669 ) ) ) )
and so on....
You can see there are controllers in a single .s file, inside a single "animation" section with 4, 2, 8 key frames defined (and there are others with 5, 6, 7 in this same file, you could check.). The FrameCount in MSTSWagon.cs will eventually be set to 8 with Math.Max() function, because there is not possible to store different FrameCount-s for the controllers with only 2 or 4 key frames. So the "key" value will run from 0 to 7 for all controllers, even for those that would need it only to run till 5. So the "key" variable needs to be protected against overrunning the actual framecount for the controller, which is not equal to FrameCount. Otherwise the animation goes back to its initial, 0 position, when it isn't supposed to.
This is why I wrote those 2 lines in my patch offering to Shapes.cs. (Just check the consist I linked, look at the behavior of the stairs below the doors when opening and closing them, with the patch applied and without.)
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Mon, 18 Feb 2013 21:49:11 GMT |
I'm sorry, I forgot to reply:
Also, re: the MSTSWagon change, the use of Math.Max on the controllers seems to result in better behaviour - you don't get a couple of seconds pause before the doors close when you press the key, because the frame only has to count down from ~3 instead of 8 or whatever it is in the overall shape.
As I see the FrameCount is set at "animation" level, so it will set to 8 at the case above, as the max value of the number of key frames inside. And the pause before the doors close is the supposed behavior, it would be a mistake shortening that, that is how MSTS also works. In the above case for example, DOOR_A is the door itself, and DOOR_C is the stair. DOOR_C should not start moving at keys 7, 6, 5, 4. It is exactly the designed behavior to stay on its place till the key counts down to 3 (from 7). So the current animation subroutine works perfectly except that little thing my patch fixes.
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Tue, 19 Feb 2013 06:41:40 GMT |
After some sleeping I see that whlite FrameCount is stored at animation level, there are still different numbers stored for different animation types. Because of this your patch for MSTSWagon.cs was really the most correct one, and mine was wrong. (But for the doors above to work correctly the modification for Shapes.cs is also needed.)
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | James Ross (twpol) |
| Date posted | Thu, 21 Feb 2013 19:58:14 GMT |
Your Shapes.cs change makes the same fundamental change to animation behaviour as I am describing; what happens currently in OR is this:
While the current frame is between two key frames, interpolate between those two. While the current frame is beyond the last key frame, interpolate as if key frame 0 was duplicated at Animation[0].FrameCount.
So in your example in comment #6, DOOR_C animates as you'd expect from 0-3, and then interpolates between key frame 3 and key frame 0 for frames 3-8. This is clearly wrong for these types of animations.
The change we're both proposing is that, after the last key frame is reached, the animation stays at that key frame.
You do this by dropping out of the controller loop but this is incorrect if there are multiple controllers and doesn't work if you start an object's animation at/beyond the last key frame. Since this is very common/generic code in OR, I'm going to fix it in a more generic manner by adjusting the code so it still goes through all the maths but does not interpolate back to key frame 0.
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | Peter Gulyas (pzgulyas) |
| Date posted | Thu, 21 Feb 2013 23:02:18 GMT |
Thank you, James, I see now you are absolutely correct. I see you are really not just hacking but delivering real solutions. :-)
Btw while digging into the other bug I saw some animation nodes with multiple controllers with different frame counts, so I realized my proposition was incorrect, but there is no point for me in changing it now, because you will surely make a lot better code. :-)
Looky1173
commented
11 years ago | Property | Value |
|---|---|
| Posted by | James Ross (twpol) |
| Date posted | Sat, 23 Feb 2013 21:01:06 GMT |
Fixed in V1456.
Imported from https://bugs.launchpad.net/bugs/1127500
The problem lies in MSTSWagon.cs class AnimatedPart incorrectly sets FrameCount in cases there are different frame counts used for different anim_node entries in a single shape file. The FrameCount is set from the first anim_node with non-zero frame count, no matter if there are other anim_nodes with higher number of keyframes.
An example vehicle (Stadler Flirt) that suffers from this problem, in case you would like to test it: http://users.atw.hu/flirt-msts/e107_files/downloads/H-Start_5341-004_10_v2.zip
I submit a patch to correct this problem, attached to this bug report. There are patches to two files need to be applied: MSTSWagon.cs and Shapes.cs.
I would like to ask to accept this patch, if you see no problems with it. Thank you very much!