Brainman

Hi!
Found a couple of very annoying bugs.
1) (Haven't found on forum) Bug related to the addition/delition of slots.
When you have animation with keyed draw order and then in setup mode you add/delete slots between other slots, which change their order during animation, keyed draw orders are not corrected.
I'm sure the root of this problem is because keyed draw order is written as slot offsets.

Example:
We have slots with draw order in setup mode:
"A", "B", "C", "D"
Then in animation they change their order so it appears as:
"B", "C", "A", "D" ("A" offset is -2, others unchanged)

Go to setup mode and delete "B" slot. Then in setup draw order will be:
"A", "C", "D"
But in keyed order it will be:
"C", "D", "A" (instead of "C", "A", "D"), because the "A" offset is still -2 instead of changing to -1.

I think (maybe i'm wrong) you decided to make keyable draw order with offsets to minimize export files. But it's much harder to monitor changes. Also it's much harder to customize a skeleton reader runtime.

If you could change keyable draw order from the "offset" system to just "writing all slots with new order in a key" (please :'( i know it's hard because of all the runtimes... ) or at least fix this bug in editor, i'd be very grateful!

2) (found here, but description there is incorrect and still not fixed) Bug related to changing multiple slots draw order.
When you select multiple slots and drop them to change order, their "inside" order is inverted.

Example:
If we have our slots:
"A", "B", "C", "D"
and we select ("A", "B", "C") and move them after "D", then draw order appears as:
"D", "C", "B", "A" (instead of "D", "A", "B", "C").

Should be easy to fix — after "drag" just invert order on "drop".

Thank you!
User avatar
Brainman
  • Posts: 14

Nate

1) If offsets are not used internally, then adding a slot would always put it at the end (or beginning) for each draw order key. With offsets, adding a slot can sort of put it where you want without requiring you to rekey all your animations. I think adding slots is more common than deleting a slot. I will fix deleting a slot by adjusting the offsets.

2) Hmm, seems you're right. Fixed it (again, for reals ;)).
User avatar
Nate

Nate
  • Posts: 10562

Brainman

1) But Spine internally will need to rekey all draw orders. Deleting and adding a slot needs this offset fix for every draw order key.

With my example:
"A", "B", "C", "D"
Changes to:
"B", "C", "A", "D" ("A" offset is -2, others unchanged)

Go to setup mode and add "B2" slot. Then in setup draw order will be:
"A", "B", "B2", "C", "D"
But in keyed order it will be:
"B", "B2", "A", "C", "D" (instead of "B", "B2", "C", "A", "D" ), because the "A" offset is still -2 instead of changing to -3.

So in either case rekeying is still needed. And you are right, offsets are needed to rekey draw orders automatically, though Spine could just calculate offsets based on "writing all slots with new order in a key" system :) But yeah, it's still the same task...

Thanks for the quick reply!
User avatar
Brainman
  • Posts: 14

Nate

What do you think of this:

Use case #1: delete slot
Setup mode: ABCD
Animation: ABCD => A-2 => BCAD
Setup mode: ABCD => delete B => ACD
Adjust animation: ABCD => A-2 => BCAD => delete B => CAD => becomes A-1
Animation: ACD => A-1 => CAD

Use case #2: move slot
Setup mode: ABCD
Animation: ABCD => A-2 => BCAD
Setup mode, add slot X: ABCD => ABCDX (new slots go to end of draw order)
Animation: ABCDX => A-2 => BCADX
Setup mode, move slot X+2: ABCDX => X+2 => ABXCD
Adjust animation: ABCDX => A-2 => BCADX => X+2 => BCXAD => becomes A-3
Animation: ABXCD => A-3 => BCXAD
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Oh, yes. I described the problem incorrectly, sorry :$ .

You see, adding a slot (use case #2) means putting it to the draw order bottom and then you can move in manually so offsets are adjusted. But copying a slot puts it right above the slot that is being copied. And i think that it does not "put the slot to the bottom and then move". So it does not adjust offsets :)
User avatar
Brainman
  • Posts: 14

Nate

Ok, the above is done in 1.6.14, including when a slot is duplicated. :clap:
User avatar
Nate

Nate
  • Posts: 10562

Brainman

I'm using 1.6.14 and it doesn't work for duplicated slots. Added 2 test projects to demonstrate.
First is simple ABCD => A-2 => BCAD
Second is same with duplicated "B" slot.
You do not have the required permissions to view the files attached to this post.
User avatar
Brainman
  • Posts: 14

DoobyDude

Didn't want to start a new thread as it's semi-related to this, but Nate are there any plans to allow us to move the animations up and down within the list in the editor?

It's more an organisational issue as with a lot of animations you cannot move their position in the list and things get really messy when looking for the correct one unless you know beforehand every single animation you are going to have.

Sorry for the thread hijack but as I said it was related (ish!) :)
DoobyDude
  • Posts: 111

Nate

Brainman, gah very sorry, I meant 1.6.15. :( Uploading it now.

DoobyDude: https://trello.com/c/m8cZ6qBd/66-groups ... ble-export
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Oh, great!
Thanks, Nate :yes:
User avatar
Brainman
  • Posts: 14

Nate

If you could try it out, I'd be interested to hear if you find it works as expected.
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Well there are some problems.

1) Deleting slots
Works almost fine, except i got some uncaugth exception one time, will try to reproduce it later
Uncaught
java.lang.ArrayIndexOutOfBoundsException: 23
at ask.a(SourceFile:710)
at zq.a(SourceFile:716)
at su.a(SourceFile:254)
at hy.a(SourceFile:89)
at jk.a(SourceFile:28)
at gt.a(SourceFile:165)
at gt.a(SourceFile:136)
at com.badlogic.gdx.scenes.scene2d.ui.Button.b(SourceFile:117)
at hu.b(SourceFile:90)
at jm.b(SourceFile:84)
at gz.a(SourceFile:57)
at hb.b(SourceFile:343)
at l.b(SourceFile:94)
at be.d(SourceFile:297)
at ar.run(SourceFile:228)
at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:251)
at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:711)
at java.awt.EventQueue.access$000(EventQueue.java:104)
at java.awt.EventQueue$3.run(EventQueue.java:672)
at java.awt.EventQueue$3.run(EventQueue.java:670)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:681)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:244)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:163)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:151)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:147)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:139)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:97)
2) Duplicating slots
Duplicated slots in keyed draw orders appear below the original slot, while in setup they're set above the original.
Like:
A,B,C,D => A - 2 => BCAD
And
A,B,C,C-copy,D => A - 3 => B,C-copy,C,A,D (C and C-copy inverted)
User avatar
Brainman
  • Posts: 14

Nate

1) That's not good. I need steps to reproduce the crash to fix this one. :(

2) Duplicate works the same as adding to the end and then moving, so it happens like this:
Animation: ABCD => A-2 => BCAD
Setup, add X: ABCD => add X => ABCDX
Animation: ABCDX => A-2 => BCADX
Setup, X+2: ABCDX => X+2 => ABXCD
Adjust animation: ABCDX => A-2 => BCADX => X+2 => BCXAD => becomes A-3, C+2
Animation: ABXCD => A-3, C+2 => BCXAD

If this isn't the desired result then how the animation is adjusted needs to be changed. We know ABCD => A-2 => BCAD, but how do we know where the X should be inserted? Keep in mind that while this example is A-2, whatever approach also needs to work if any or all slots have changed order.

After X+2 we have ABXCD so we know X has neighbors B and C. We could insert it between B and C:
ABCD => A-2 => BCAD => insert X between B and C => BXCAD
However, with different keys in the animation, B and C may no longer be next to each other.

In the last step of adjusting the animation, we could just drop any changes that weren't originally in the animation. So, we'd drop C+2 and end up with just A-3. This works for this simple example, but I'm not sure it works for others.

I have a feeling there is no solution. We can't know where the user intends the slot to be inserted in animations that have changed the slot order all around. We may be able to do something reasonable though. Open to ideas.
User avatar
Nate

Nate
  • Posts: 10562

Brainman

1) As for uncaught exception. Found a way to reproduce, though i don't fully understand why it happens.
Added simple test project in attachment. To cause an exception, simply delete slot "A".

2) In your example i don't understand, why it "becomes A-3, C+2".
After adding X we get final picture in setup mode:
ABXCD
Previous keyed order was:
BCAD
So where X should be in new keyed order:
BXCAD
And it's indeed only A-3.

Even looking just at this:
Adjust animation: ABCDX => A-2 => BCADX => X+2 => BCXAD => becomes A-3, C+2
Final transformation is:
ABCDX => BCXAD
If we apply (A-3, C+2):
ABCDX => A-3 => BCDAX => C+2 (+2 is out of bounds) => CBDAX
And this does not look like needed result.

Possible adjustments are:
ABCDX => (A-4, D-2) or (D-1, A-3) => BCXAD
It all depends on offsets applying sequence.

As for ideas on how to change the adjusting process.
With our ABXCD example:
Transformation ABCD => add X => ABCDX => X+2 => ABXCD can only be applied to the original state to be correct.
Because applying (A-2) in keyed draw order before (X+2) ruins the X position:
ABCDX => A-2 => BCADX => X+2 => BCXAD (But X already should be +3 and not +2 because A sort of "fell in range" of X offset)
The other possibility of transformation is apply (A-2) after (X+2):
ABCDX => X+2 => ABXCD => A-2 => BXACD (But A already should be -3 and not -2 because now the X sort of "fell in range" of A offset, which leads to the exact same problem)
So the solution is to somehow analize situations where the new slot "falls in range" of the old transformations and adjust them accordingly.

Jeez, lots of mind-blowing examples... :bang:
You do not have the required permissions to view the files attached to this post.
User avatar
Brainman
  • Posts: 14

Nate

1) Thanks, I fixed the crash when deleting some slots.

2) For various reasons, the slot offsets aren't a list of changes to be applied in sequence. Instead, the offsets are relative to the setup pose index and are used to rebuild the modified draw order. It is not as straightforward to humans as sequential change list, but it's where I arrived after exploring problems with a number of alternatives.

With ABXCD to apply A-3, C+2 you place A at its setup index minus 3, so 4-3=1 and you have ???A?. Next you place C at its setup index plus 2, so 1+2=3 and you have ?C?A?. Now you fill in the rest from the setup pose order to get BCXAD.

At any rate, the offset notation isn't too important. What is really important is the approach used to adjust animations when a slot is added.

I think your assessment of why we don't get the desired results is correct. The range that affects placement of X is its old index to its new index. I've written code that adjusts the movement of X if any animation changes start outside and end inside of this range. It seems to work, can you give it a try in 1.6.18? Uploading now.

Adjust animation: ABCDX => A-2 => BCADX => X+2, +1 because A moves inside range => BXCAD => becomes A-3

Thanks for all your help with this! :)
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Whoa, great work!

The X position is not always the desired one, but you must be right that there might be no absolute solution to this. Still, it's far better than it was previously, all other slots now lay where they should in keyed orders.

Thanks, Nate.
User avatar
Brainman
  • Posts: 14

Nate

Sweet! :D If you have some scenarios where X is in the wrong place, maybe it can be fixed. There might be an off-by-one issue somewhere, where < gets used instead of <= or whatever. :)
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Okay, here's another example scenario in attachment :)

While old slots are moved correctly, duplicated D-copy slot just will stay on it's original spot though it should be between C and D slots.
You do not have the required permissions to view the files attached to this post.
User avatar
Brainman
  • Posts: 14

Nate

Nice! My range check was backward when moving a slot the other direction. That fixed this test case, thanks! :) It'll be in 1.6.19, uploading now.
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Well, yeah. But there's another case when it doesn't work :D
For example, if we duplicate B slot, not D :)
(Attached another test)
You do not have the required permissions to view the files attached to this post.
User avatar
Brainman
  • Posts: 14

Nate

Gah. :( Too many edge cases with the approach that detects changes inside the range of a moved slot. I've come up with a new approach that handles all the scenarios in a much better way. Can you give it a shot in 1.6.20? I will try to upload it in 2-3 hours.

One caveat is that if a slot has been keyed, its order for that key won't be affected if you change the order in setup mode. This is usually what you want, after all you specifically moved the slot -- except which slots have offsets is determined by a fancy diff algorithm and diffing is ambiguous. Eg, ABC -> ACB could be described as B+1 or C-1.

Still, I think the new way is quite robust and I'm pretty sure we won't be able to find a better way to keep animation draw order keys intact while adding, duplicating, deleting, or changing the setup pose order.
User avatar
Nate

Nate
  • Posts: 10562

Nate

Sorry for the delay, 1.6.20 is up.
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Sorry for the long absence.
New order system is indeed better and works properly.

...
Except... :giggle: (oh no, not again!)
Well, it's only the visual part, nothing serious. Because of the offset system, slot-up and slot-down icons are showed incorrectly. I'm adding another test project — like the old ones, except B and C slots are swapped in keyed order. And C slot is shown as "goes downward" while it does go upward.
You do not have the required permissions to view the files attached to this post.
User avatar
Brainman
  • Posts: 14

Nate

:bang: When well it end!? :p Fixed this in 1.6.25.
User avatar
Nate

Nate
  • Posts: 10562

Brainman

Looks like it is over, finally :)

Thank you for tons of work!
User avatar
Brainman
  • Posts: 14


Return to Bugs