Netplay backward compatibility temporary fix

[barbudreadmon]

@Cthulhu-throwaway

Follow-up of https://github.com/libretro/RetroArch/pull/14101

For now i applied https://github.com/libretro/FBNeo/commit/4af4610ae8865cc278fc32c2ccc891b38a33c537 so things should be back to normal.

Please let me know when you think it's time to revert this temp fix.

Also, i might be wrong, but i think savestates were supposed to be part of rollback savestates, whether RETRO_ENVIRONMENT_GET_SAVESTATE_CONTEXT or RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE was used, because nAction |= ACB_NET_OPT; was used in both cases, so the only difference should have been about whether they were flushed to disk or not, which should be controlled by EnableHiscores.

So there might be some misunderstanding about the real cause of the issue. I have 2 theories and i'd be interested to hear how cps3 games are faring after this temp fix, since iirc they use the only other savestate codepath checking ACB_NET_OPT (to save bandwidth).

[barbudreadmon]

I also just noticed i forgot to use https://github.com/libretro/FBNeo/commit/de6ac7b00681c07f38e735a49f56b61ce0e3644c in the new RETRO_ENVIRONMENT_GET_SAVESTATE_CONTEXT codepath. Turning on this value will prevent having different computed timestamp on some emulated systems.

I don't think that was your problem here, since your error was reporting mismatching sizes, but that'd certainly cause issues, at the very least for neogeo.

[ghost]

I'll test this once some lobbies running RetroArch stable start popping in. I don't have any CPS-3 games though.

[barbudreadmon]

I have been thinking about this for the past few hours, and that might be what happened, there is a major issue with sharing hiscores over a netplay session: the hiscore.dat file needs to be present on both sides, or the computed savestate sizes can't match.

I'm probably gonna remove hiscore saving for good in netplay session, because i can't think of a way to deal with mismatching hiscore.dat setups

[ghost]

It's not a big loss. Keeping the hiscores of the host would be just a bonus and most people won't even make use of the netplay saves anyway.

[barbudreadmon]

Ok, hiscore.dat support will stay disabled in a netplay context then. We can close this.

[ghost]

@barbudreadmon updated to nightly again and still getting:

[ERROR] [Netplay] Netplay state load with an unexpected save state size.
[INFO] [Netplay] Netplay disconnected

EDIT: Compiling a custom edit of RetroArch to give you these mismatched sizes...

-> Game: mslugx [INFO] [Netplay] Expected: 414111, Got: 414033, Raw: 32557, Buffer Size: 828222

Raw refers to the savestate data as it is in the TCP stream, which can be compressed. Buffer Size refers to the buffer that holds that raw data.

Error can be triggered on:

expected != got or raw > buffer.

Debug code:

            RECV(&state_size, sizeof(state_size))
               return false;
            state_size     = ntohl(state_size);
            state_size_raw = cmd_size - (sizeof(frame) + sizeof(state_size));

            if (state_size != netplay->state_size ||
                  state_size_raw > netplay->zbuffer_size)
            {
               RARCH_ERR("[Netplay] Netplay state load with an unexpected save state size.\n");
               RARCH_LOG("[Netplay] Expected: %u, Got: %u, Raw: %u, Buffer Size: %u\n",
                  (unsigned)netplay->state_size, (unsigned)state_size, (unsigned)state_size_raw, (unsigned)netplay->zbuffer_size);
               return netplay_cmd_nak(netplay, connection);
            }

[barbudreadmon]

414111 is the correct savestate size for mslugx without including hiscores on my computer, and what i'm returning through retro_serialize_size for netplay whatever environment variable is in use.

There could be discrepancies from different core options, like different bios or whatever, but i've no idea where this 414033 value could be coming from, it doesn't look like something that could result from the environment callback changes, is this from your host or the other host ?

If this is from your host, could you build the core using make -C src/burner/libretro/ DEBUG=1 and send me the logs ?

[ghost]

414111 is the correct savestate size for mslugx without including hiscores on my computer, and what i'm returning through retro_serialize_size for netplay whatever environment variable is in use.

There could be discrepancies from different core options, like different bios or whatever, but i've no idea where this 414033 value could be coming from, it doesn't look like something that could result from the environment callback changes, is this from your host or the other host ?

If this is from your host, could you build the core using make -C src/burner/libretro/ DEBUG=1 and send me the logs ?

It's from a host that I usually play with. I could connect to him with the fbneo core included in 1.10.3 stable, but not with the nightly compiled at this commit: https://github.com/libretro/FBNeo/commit/94be34def54b2746114bfd287abdfadd97e36d5d

[ghost]

@barbudreadmon Just tried two instances at the same machine and config. One running the core from 1.10.3 stable (8881c2f) and another running the core from nightly (94be34d).

Result (nightly connecting to stable):

[ERROR] [Netplay] Netplay state load with an unexpected save state size.
[INFO] [Netplay] Expected: 414111, Got: 414033, Raw: 19185, Buffer Size: 828222

Commit 8881c2f might be a good place to start looking at.

[barbudreadmon]

I confirm mslugx savestates from 2 months ago had a size of 414033, but that change in size has nothing to do with the environment callback changes. You shouldn't expect a savestate from 2 months ago to still work in today's build, you shouldn't even expect a savestate from yesterday's build to still work in today's build. There are commits everyday, and some of them affect savestates.

What matters here is that the same version of the FBNeo core can be netplayed between different versions of retroarch.

If this is what this is about, can i close ?

[barbudreadmon]

The size change comes from that savestate fix for various emulated sound boards : https://github.com/libretro/FBNeo/commit/2fc941ef54cf31f76b8246eb8c3e39435c33d8ed

[ghost]

This is really a problem, if already well-supported games can still get changes in their savestates.

Here are a couple of pitfalls with this:

1. The only change in version is the commit's hash, which most users don't know about. As far as they are concerned, 1.0.0.03 is still 1.0.0.03.
2. Platforms which don't get their cores updated directly from nightly (Steam as an example).
3. For clients, the generic "update your cores" advice won't do any good, if the host doesn't do the same.

The worst part is that I can't do anything on RetroArch's netplay side other than maybe add a notification if the first load savestate fails (telling them it might be a core incompatibility).

Having a stable repository for cores is an idea to really think about (and have it as the default path in the cfg). Issues like these make certain less-knowledgeable users just believe and spread that RetroArch's netplay is worthless.

[ghost]

I'll open an issue on libretro/RetroArch for the possibility of having a stable repository for cores.

[barbudreadmon]

This is really a problem, if already well-supported games can still get changes in their savestates.

The state of the sound board wasn't properly saved, the same sound board can be used in dozens of different arcade systems, in different ways. There was a bug and it had to be fixed, even if that bug didn't necessarily affect all games using that sound board.

Having a stable repository for cores is an idea to really think about (and have it as the default path in the cfg).

Well, just to be clear, if libretro users getting stuck using outdated versions of the core just for the sake of netplay becomes the norm, we'll stop supporting libretro users. We don't want to be bothered by users reporting issues that were already fixed. I have enough problems with users complaining about missing support for game XXX because they are using some outdated android versions from google play.

[barbudreadmon]

The worst part is that I can't do anything on RetroArch's netplay side other than maybe add a notification if the first load savestate fails (telling them it might be a core incompatibility).

Users should get a warning notification if the library_version retrieved by retro_get_system_info don't match. Even if the savestates are compatible, there is no guarantee a game will be emulated the same if users have mismatching emulators.

[ghost]

There is already a warning for that, but it's generally ignored because different commit hashes will trigger the warning and most of the time (95% of the time or so), netplay will work correctly. https://github.com/libretro/RetroArch/blob/494c93df3eea73b8019931f19138b1f1be05b98c/network/netplay/netplay_frontend.c#L1600-L1609

As for not wanting a stable repo for cores, well, there must be a compromise here. RetroArch's netplay is nearly dead and at some point, if things continue like this, I'll lose interest and netplay will be back at having no developers and stuck in development limbo. Besides, a stable repo won't prevent you from using nightly builds, you can just change the path from stable to nightly, like you can do right now.

Expecting users to use their brains and always do a core update before netplay is expecting way too much. Take a look at this screenshot of the lobby (look at the RetroArch versions): https://i.imgur.com/rbQV2uF.png This is already enough reason to piss me off, but then you also have the "use parsec, bro" crowd, all because of these issues followed by their stupidity.

The lack of version increments with those changes also make it really difficult to track those problems; had the version been incremented, I would've correctly assumed a version incompatibility for the 414111/414033 savestate size issue above.

[barbudreadmon]

Again, we can't be bothered by users thinking they are using the latest version of this emulator while they don't, this is already enough of a problem with steam builds, google play builds, ....

So i won't change my mind about this, if this becomes the norm, i'll have to stop providing support on libretro channels (github, forum, discord, ...).

[barbudreadmon]

@Cthulhu-throwaway Also, in case you don't know the background behind this core, we get quite a few reports/requests from libretro users, and most of the time the user can enjoy what he asked for the same day. Your suggestion is basically destroying this relation, which is why i can't see this as an acceptable suggestion as far as providing support is concerned.

The lack of version increments with those changes also make it really difficult to track those problems; had the version been incremented, I would've correctly assumed a version incompatibility for the 414111/414033 savestate size issue above.

And obviously, incrementing version every time there is a change is unrealistic and wouldn't help anyone when you make 400 commits just over the past 2 months.

[MistyDreams]

And obviously, incrementing version every time there is a change is unrealistic and wouldn't help anyone when you make 400 commits just over the past 2 months.

checked fbneo standalone was only 233 commits. Maybe the merging and updating dats added the extra commits for you. That aside people have alternatives if you choose not to embrace netplay in a way that works for the platform.

No one will force this on anyone RA has features which ones you want to embrace is up to you at the end of the day as dev. Im sure some people will be disappointed but they will get over it or just use something else that makes does netplay a priority. Better to keep everyone happy than have half working things at the end of the day.

[barbudreadmon]

was only 233 commits

Does that make a difference ? As i said incrementing version X times a day is unrealistic and wouldn't help anyone.

if you choose not to embrace netplay

What a fucking ridiculous statement, we have applied netplay fixes for years. What was asked here was prioritizing netplay users over our base users, which we can't agree with since it'll prevent us from providing proper support for all our users.

just use something else that makes does netplay a priority

And those other solutions still expect the users to update them every time there is a new release. Sorry for providing new releases every day.

Even if you proceeded with that "stable version" thingy, it wouldn't solve anything, there would still be people who wouldn't update to latest "stable version". As a matter of proof, the 2 versions used as a basis for your argument here are already 2 months apart. Or are you telling me it's actually your intent to update your so-called "stable versions" only once every few months/years ?

Your solution is not a good one, it's not solving the real problem (users not updating their cores) and only transforming support into a very painful experience for us and our users.

[MistyDreams]

Im choosing to ignore your language and you behaviour in general. You seem to think your way is the only way and no insight to the big picture of netplay implementations should work better for a user base as a whole. Good luck with your endeavours. I dont wish to deal emotionally unstable people that have outbursts of swearing.

[barbudreadmon]

I dont wish to deal emotionally unstable people that have outbursts of swearing.

@MistyDreams That's rich coming from someone who came trolling by telling falsehoods in 2 consecutive threads.

There is no "big picture" here, this suggestion is just screwing the experience of the devs and most users for the sake of arguably benefiting the netplay users, at the condition they don't forget to update their cores (and we already know they will) to latest "stable version", which is also not guaranteed to be a stable version by any means.

This is not a proper solution. If you want to force a version for netplay, then find a way to really force it for netplay, without interfering with the experience of non-netplay users. I don't know, maybe create an archive of core snapshots and enforce client-side to download and use the closest match to the host-side when connecting to it.

[MistyDreams]

As ive said before. Im not trolling I just dont agree a commit is a release versioning is. What falsehoods are you even talking about or you creating a narrative to argue with me.

I dont think we will see eye to eye accusing people that do not agree with you doesnt make them a troll. There is a bigger thread on the issue where I hope many others expand there thoughts and make something better. It would do you well to air you options there and help the project as whole rather than picking fights with people that dont agree with your views.

I should have ignored you on github on the last post I will do this now to avoid escalation people like you are best avoided

[barbudreadmon]

What falsehoods are you even talking about

When you say to someone that he didn't do something while he actually did, that's called falsehoods. Closing as this is not a FBNeo problem anymore