Thanks for digging into this! The order meta does need to be updated/calculated at least when adding a new lesson, and that was moved to the new structure class, but obviously something is not working correctly.

@yscik yes, obviously. :) I understand that in case of a new lesson a meta needs to be generated, but that only needs to be done in the meta of the new post. When updating a post, even that isn't necessary, since there is no way that action will influence the lesson order.

I'm sure there are ways to determine the highest lesson order number for a new post, and even when not, the repercussions of having two lessons with the same lesson order-meta isn't that severe. Often the content creator will reorder the lessons after adding a lesson anyway.

Hope there are enough clues now, so it can be fixed soon.

@yscik
I don't know if this is the exact cause, but in digging further in this issue I looked at where the lessons are ordered in the structure and came across Sensei_Course_Structure::sort_structure.

If I understand this function correctly, it is meant to sort the modules/lessons structure of the course to match the $order that has been passed to it. This $order as far as I have looked is generated by the lesson order admin page (or also by the structure block?). But it is empty ($order_string parameter) when the current page is the lesson editor.

In case a lesson is saved, that $order is an array containing only one entry with the value of 0. That one entry is generated since $order_string passed to Sensei()->admin->save_lesson_order() is empty, but since no id matches it, all calls to the anonymous callback of usort() return the value 0

I even tested that by hardcoding the return 0 as first statement, but otherwise it is returned after:

$a_position = array_search( $a['id'], $order, true );
$b_position = array_search( $b['id'], $order, true );

// If both weren't sorted, keep the current positions.
if ( false === $a_position && false === $b_position ) {
    return 0;
}

Perhaps that is expected to leave the order as is (also given the comment in the above code), but I find that it doesn't.

When the structure is passed into it, it has already been sorted to the lesson_order in the lessons meta ( '_order_' . $course_id ) giving any new lessons a order meta of 100000, but after it has been passed through Sensei_Course_Structure::sort_structure that order has been changed, even though it shouldn't.

In the php documentation see a note:

Note:

If two members compare as equal, their relative order in the sorted array is undefined.

And I'm wondering what undefined means in this context. It might mean the result is that the order may or may not change in that cases. Here all members compare as equal (since the result of the callable is 0 in any case) so php might not find it important to keep the order it started with. Since equal values may be switched without affecting the order.

Another note says that usort() may also change existing keys.

Note: 

This function assigns new keys to the elements in array. It will remove any existing keys that may have been assigned, rather than just reordering the keys.

When I log the values on our system ( I extracted only the id's from $structure here, to make it easy to compare visually ) I see the difference between what the function accepts and what it returns:

The current method is: Sensei_Course_Structure::sort_structure
The current filter is: save_post
Accepted values at the start of the function
The value of $structure is: 
'90597, 89981, 90645, 90319, 90876, 90318, 89961, 90307, 90309, 90192, 90197, 90598, 90184, 90584, 91101, 90651, 90190, 90217, 90306, 90294, 90295, 90296, 90283, 90285, 90284, 90277, 90278, 90276, 90475, 90478, 90482, 90486, 90267, 90257, 91244, 90255, 90256, 90258, 90604, 90242, 90243, 90241, 90240, 90234, 90230, 90308, 90661'
The value of $order is: 
array (
  0 => 0,
)
The value of $type is: 
'lesson'

2021-02-01 17:38:02 :: 
The current method is: Sensei_Course_Structure::sort_structure
The current filter is: save_post
Values just before returning $structure after sorting it.
The value of $structure is: 
'90597, 90255, 90278, 90276, 90475, 90478, 90482, 90486, 90267, 90257, 91244, 90256, 90284, 90258, 90604, 90242, 90243, 90241, 90240, 90234, 90230, 90308, 90277, 90285, 89981, 90598, 90645, 90319, 90876, 90318, 89961, 90307, 90309, 90192, 90197, 90184, 90283, 90584, 91101, 90651, 90190, 90217, 90306, 90294, 90295, 90296, 90661'
The value of $order is: 
array (
  0 => 0,
)
The value of $type is: 
'lesson'

You see that at those two markers, the $structure is different. So I'm thinking this is the problem, although it doesn't change that a lot of code has been executed in a situation that no change should even have been considered. When $order_string is empty ordering seems redundant and probably $order should be an empty array.

An easy fix might than be to make the call to Sensei_Course_Structure::sort_structure conditional on the $order value and skip it when $order is empty (in this case that means array ( 0 => 0, ).

But it might be that usort() isn't the best solution for this functionality anyway, given that a return 0 from the callback affects the initial order.

I've tried making the Sensei_Course_Structure::sort_structure call conditional like this to skip the sort_structure call for lesson pages.

if ( ! empty( $order ) 
&& $order !== array( 0 ) ) {
    $course_structure = Sensei_Course_Structure::sort_structure( $course_structure, $order, 'lesson' );
}

Not tested enough to be absolutely certain it solves the complete problem, but it might. Calling sort_structure in this cases is redundant anyway and knowing there is a problem there, makes it important to skip the call.

@yscik
I've issued a PR for this issue. Perhaps it is enough. For us it seems to do the trick. The lesson order is now retained at both course save and lesson save.

Instead of making the call to sort_structure conditional I decided to wrap the usort call in a condition. I have seen sort_structure is called in several places and this way any other calls that pass an empty $order parameter will be corrected too.

Hope it helps.

Thank you @hansschuijff, that's some great research! I'll check the PR and we can get this out there.

@yscik I was thinking some more at why usort() failed in the first place. The PR I issued was just skipping of that problem. The problem as I said earlier is that returning 0 implies the order doesn't matter. So today I tested a bit more and issued a new PR to also address that problem. The basic problem is trying to sort information that isn't available in the callback.

You find my thoughts and solution in PR3957.