Replay filter broken

[Langbart]

When checking the replay screen in the latest SVN version [25658], the Filter was enabled, but there were still some gray replays displayed.

I will attach the my replay folder as a zip file - 0.0.25.zip

A short discussion about the problem can be found in IRC 0ad-dev (3/Jun/21)

18:28 < Angen> Langbart: yeah, something is wrong with filter
18:29 < Angen> it sometimes does not filter some replays
18:29 < Angen> and sometimes it filters all

Edit 1: The problem probably starts with changeset [25634], but errors are displayed until it is fixed in [25651]. This means that the problem is probably related to [25634], but you can only check it with [25651] for the first time.

[Imarok]

Hmm, I can't reproduce the issue with your data. Did you have mods enabled?

[Langbart]

Replying to Imarok:

Hmm, I can't reproduce the issue with your data. Did you have mods enabled?

No, the problem is still there with the latest SVN version [25758], but the replay filter works perfectly with the ​recently released macOS bundle for A25.

PS: I used the same replay folder to test the macOS bundle and the SVN version.

[Freagarach]

It may or may not be related, but when I return from a replay it doesn't show any replays at all, unless I untick and tick the filter. r25763

[wraitii]

can't reproduce any issue.

[Imarok]

Replying to Freagarach:

It may or may not be related, but when I return from a replay it doesn't show any replays at all, unless I untick and tick the filter. r25763

I also can't reproduce that. I guess that means that you have to investigate the code :/

[Langbart]

Replying to Imarok:

I guess that means that you have to investigate the code :/

Good conversation on ​IRC 0ad-dev (11/Jul/21) between me and Stan, the problem boils down to the changes from ​changeset 25634 (2/Jun/21) with the introduction of ignoreInCompatibilityChecks.

[Langbart]

Additional observations added to the description for changeset [25633], [25634] and [25651].

[Silier]

Looks like isReplayCompatible has real issue

let iscmp = isReplayCompatible(replay);
let iscmp2 = isReplayCompatible(replay);
let iscmp3 = isReplayCompatible(replay);

if (iscmp != iscmp2 || iscmp3 != iscmp2)
   error("isReplayCompatible has real issue");

it returns different results for the same replay, dont know why yet. Also modToString has issue returning undefined, undefined

[Silier]

if (mod.name === undefined)
   warn(uneval(mod));

results in

WARNING: ["public", "0.0.25"]

But this is irrelevant for release because in a25, all a24 replays will be incompatible by engine version and not mods and this happens only with old replays

[Silier]

WARNING: [["public", "0.0.25"]] vs [{mod:"public", name:"0ad", version:"0.0.25", ignoreInCompatibilityChecks:false}]

WARNING: hasSameMods?true

WARNING: [["public", "0.0.25"]] vs [{mod:"public", name:"0ad", version:"0.0.25", ignoreInCompatibilityChecks:false}]

WARNING: hasSameMods?true

WARNING: [["public", "0.0.25"]] vs [{mod:"public", name:"0ad", version:"0.0.25", ignoreInCompatibilityChecks:false}]

WARNING: hasSameMods?false
}}}}

[Silier]

[["0ad", "0.0.25"]] vs [{mod:"public", name:"0ad", version:"0.0.25", ignoreInCompatibilityChecks:false}]

code is very inconsistant

[Stan]

Maybe it bugs because public != 0ad ?

[Silier]

it checks for path, what means mod in this case. so as you can see above, in the third call it decided that they are not compatible despite it passed twice. that said, no idea how 0ad got there instead of public.

I would gladly say, wontfix or worksome because in releaase: all replays will have old version number so checks will fail in engine version, not reaching mod compatibility checks, unless someone provides svn replay before mod rewrite which is doomed to not work anyway.

however there is something very strange going here and it is possible something else lies hidden and may cause bugs even for replays created in release

[Stan]

0ad is in the mod.json I believe wraitii implemented something just to read that. It's buggy enough to be worth investigating but i'll move on after fixing translations if you think that:s okay

[Silier]

I meant this [["0ad", "0.0.25"]] should not be happening as there supposed to be [["public", "0.0.25"]] in replays

[Stan]

Well technically it's more correct as folders should be irrelevant...

[Silier]

Techically maybe yes, but not from perspective of current code

[Freagarach]

Can it be that the check alters the replay data? (Idea courtesy of elexis.)

[Silier]

I thought about that also, but could not prove it yet

[elexis]

If deepfreeze(replay) works then statements trying to modify the object will throw an exception, refs #4257, #3647, ...

[Stan]

Maybe not the replay itself but the cache ?

[Freagarach]

Pushing back.