ruffle-rs / ruffle

A Flash Player emulator written in Rust
https://ruffle.rs
Other
15.7k stars 817 forks source link

Onda Educa: Tile randomization not working #4623

Closed sombraguerrero closed 2 years ago

sombraguerrero commented 3 years ago

Describe the bug The tiles of the memory matching games are supposed to randomize positions, but in Ruffle they don't. This of course allows students to memorize the puzzles, which is counterproductive.

Expected behavior Tiles should be able to randomize positions.

Web builds: Extension and Self-Host

What platform are you using? Self-Host with Chromium and Mozilla based browsers.

Tagging #1182 for reference, but I think this problem is a little more nuanced. These games are rounding the result of Math.random(), and that is being used in an anonymous callback function to array.sort(). This would indicate to me that at one point in time, floating point numbers greater than one were expected from Math.random(), not values between 0 and 1. Furthermore, since the anon function has no arguments, looking at Ruffle's code, I would assume it's just going to stick with default sorting behavior, which wouldn't accomplish anything useful, nor would setting the sort behavior to any other mode even if the code does respect the 0 or 1 as a bitflag.

bingo.zip

adrian17 commented 3 years ago

I think it's simpler than that and just that Vec::sort_unstable_by doesn't behave as we would expect when we pass it Ordering::Greater (I think it's because it passes values to the callback in opposite order, so the ordering won't change unless we pass it Ordering::Less at least once).

("simpler", as in the cause is simpler, but might be actually harder to fix)

    let mut v = vec![1, 3, 2, 4];
    v.sort_unstable_by(|a, b| {
        println!("{:?} {:?}", a, b);
        Ordering::Greater
    });
    println!("{:?}", v);

Prints:

3 1
2 3
4 2
[1, 3, 2, 4]

Note the opposite order of a,b.

technically this is a bug in the game, as they relied on the implementation detail of how Array.sort happened to work; but for bug compatibility, this of course needs to be fixed. Not sure how though, aside from just reimplementing Flash's sort algorithm.

adrian17 commented 3 years ago

The lazy fix would be to reverse the arguments given to the sort callback (and then reverse the result), I'm just not sure if that's guaranteed to fix is completely... Will probably fix shuffling, but if someone relied even harder on sort implementation, it might not be enough. Also vulnerable to Rust changing their implementation details too.

Herschel commented 3 years ago

Yea, this is one of those implementation details where we do it the quick+easy way using the std lib at first, knowing we'll have to reverse engineer and reimplement it ourselves at some point to get full compatibility. (Number->string and string->number conversions are another case of this).

I wonder if AVM1's sort is the same or a similar algorithm to avmplus's sort.

sombraguerrero commented 3 years ago

but that's where I'm really puzzled as to what the best approach is, because again, there are no arguments passed to the callback function at all, so there's like no flexibility to be had anywhere. So, that is to say, I also agree with you Adrian, that technically, while this was a clever thing to do in the old Flash world, it's not really great for compatibility going forward.

That's what I was wondering too, Mike, because that is a super nuanced detail for any Flash programmer to know, so I fee like either that was well documented somewhere in the past or sort was different earlier in its life.

adrian17 commented 3 years ago

because again, there are no arguments passed to the callback function at all, so there's like no flexibility to be had anywhere

They exist (Rust passes them), the AS function just ignores them. Either way, this doesn't change what I wrote above.

adrian17 commented 3 years ago

that is a super nuanced detail for any Flash programmer to know, so I fee like either that was well documented somewhere in the past

Not really, this is the kind of thing I imagine was either found by random experimentation or just passed around as a snippet. To this day you can find similar snippets for JS:

[1,2,3,4,5,6].sort( () => .5 - Math.random() );
sombraguerrero commented 3 years ago

Point taken, so it's not terribly uncommon for it to be used this way. Is it worth trying something like reversing callback arguments? To me, it seems like only fully implementing the original sort would avoid further convolution.

sombraguerrero commented 3 years ago

So, now that my brain has had some time to digest this bug, I now understand that what Onda is doing, is they're creating partitions, and they're randomizing the sort order, banking on the fact that at least one of the partitions will end up different and merge them back together. Again, I agree, some may consider this clever, but it's a very implementation-dependent and, in my opinion, not a particularly effective way to shuffle an array. So would it still be worth doing the quick and dirty fix of reversing the callback arguments? To Adrian's point, it doesn't feel like that would be in the best interest of other people's consumption of sort.

sombraguerrero commented 3 years ago

So after discussing this further with Javier, I now know that some activities are fine randomizing in this slightly "cheating" fashion, and others are not. The first example SWF I posted is Bingo from the "Oposiciones" section, but here I will attach and example that is Bingo from the "Fonemas" section, which works. I more just wanted to add this to make sure I understand our prior discussion In the Fonemas Bingo, the same thing is being done, but only on one array, so that would be consistent with what we described, wouldn't it? Because if we only do this once, and Math.round(Math.random()) happens to hit Ordering::Less, which it would most of the time, the order would change. Whereas if you perform on three arrays like Oposiciones does, there's a greater chance you're just going to hit Ordering::Greater and not have any movement.

Do I have that right? fonemas_bingo.zip

sombraguerrero commented 3 years ago

I'm actually willing to close this issue now unless people want to continue exploring it. I've done a proof of concept by editing one of the affected SWFs via JPEXS, proving that this problem can be solved by simply implementing Fisher-Yates.

sombraguerrero commented 2 years ago

Reopening this after considering that the risk of fidelity loss via editing ActionScript in JPEXS is not worth exposing Onda to unnecessary problems.