Zarel commented

Trac is apparently too broken for me to upload attachments, so for now, refer to:
http://pastebin.ca/1754094

anonymous commented

Why is #4 even a part of this patch? It is not related at all.

All the replies from your previous ticket are not even handled and are still valid points that you have not addressed in the original ticket #688

Changed 7 months ago by Per
IIRC the reason we removed autoload was that it generated lots of bogus bug reports when people forgot to mention (to other players, and in bug reports) that they played with them in online games. Before we re-enable this functionality, we need to prevent players from being able to join a game with the wrong mods, and add the list of mods used to the crash dump report. IMHO.

Changed 4 months ago by Buginator
Just a note, when we check the mod, we should calculate a CRC checksum (we need GPL code for this) of some kind, and I guess do that for each mod, then transmit it for each mod, or should we XOR all the CRCs of all the mods, and just send it then?
Whatever we decide, we need a well defined way to handle this, and so far, we don't have it, and I am strongly against doing it the haphazard approach that won't solve the issues with having mods in the first place.

Changed 4 weeks ago by Per
I think we need a better way to inform the user of why he cannot join games (because there is an autoload mod in his folders) before this is committed.

Comparing loaded mods is no longer necessary, as Buggy's datahash-checking code, committed in 2.2.4, already makes sure any incompatible mods can't be loaded.

Someone also screwed up the datachecking code since it doesn't work correctly anymore.
http://forums.wz2100.net/viewtopic.php?f=4&t=4476

http://forums.wz2100.net/viewtopic.php?f=4&t=4470

people are still going to enter games and be kicked out and then the host will have to end current game and host again.

Consider this a objection, it is a bad implementation and bug prone for 2.3 and does not address the main problem of people joining games when they shouldn't be allowed to even enter that game in the first place.

It doesn't seem like you even read what those guys talked about.

The current way at least forces everyone to know what mod they are using.
The host and users have no idea what mod is loaded with your patch until it is too late for them to do anything. That is what I mean by bad design. Your patch is worse than the current situation.

This is fine for trunk but not 2.3

Zarel commented

Replying to Warzone2100/old-trac-import#1415 (comment:2):

anonymous

With whom do I have the honor of conversing?

Why is #4 even a part of this patch? It is not related at all.

It's semi-related. I included it mostly because it needed feedback as well, and I am also planning on committing that change to 2.3. So feedback, please.

All the replies from your previous ticket are not even handled and are still valid points that you have not addressed in the original ticket #688

Apologies, I wanted to keep the mod as minimal as possible. Here, how's about this version?

http://pastebin.ca/1754341

It adds:

5. Grays out games with incompatible mods in the NetPlay game list, and gives a warning in the tooltip: "Your loaded mods are incompatible with this game. (Check mods/autoload/?)"
6. Gives a warning when a game with an incompatible mod is clicked: "You have an incompatible mod."

Again, I'm planning on committing to trunk, allowing a period for testing, then backporting to 2.3.

Someone also screwed up the datachecking code since it doesn't work correctly anymore.
http://forums.wz2100.net/viewtopic.php?f=4&t=4476

http://forums.wz2100.net/viewtopic.php?f=4&t=4470

Well, then, that will presumably be fixed before 2.3.0 is released, so it's irrelevant.

Consider this a objection, it is a bad implementation and bug prone for 2.3 and does not address the main problem of people joining games when they shouldn't be allowed to even enter that game in the first place.

It addresses the main problem now.

Now tell me why this is a bad implementation and why it is bug prone. Such vague accusations from an anonymous source really don't mean much, neither as an objection nor as useful criticism.

Terminator <wz2100@...> commented

I think its good improvement. At least noobs now will see what is the problem when playing MP games.

I understood that MODs list will be included only in a HOSTED game version, in lobby. Right ?
May I recommend to add this MOD list to client game version(in the bottom right corner of the screen). So MODs will be not only in lobby list, but will be seen by player all the time.

Zarel changed status from new to accepted

Zarel set owner to Zarel

Zarel edited the issue description

cybersphinx commented

About #4, if you want to discuss that, hiding it behind an unrelated label won't help there. I don't really like the approach of hardcoding more and more stuff.

For real mods, some indication on the hosting screen would be nice, like "Autoloaded mods: abc, manually loaded: yzx. Note that all players need to have the same mods to join your game." Especially important for autoloaded mods people might forget about.

Leads to a second thing, distinction between autoloaded and other mods.

Not sure I like the concept of autoloading itself, since it leads to what was mentioned above, if you want to host a game others can easily join, you need to quit, remove mod, restart, and that makes the whole thing somewhat less useful, in the long run, it's easier to just create the right shortcuts for mods.

Adding the lists to the crashdump should be easy, but is also missing yet.

-                      txt = _("Wrong data/mod detected by Host.");
+                      txt = _("You have an incompatible mod.");

Are you sure this can only happen when people use mods now?

Zarel edited the issue description

Zarel commented

Replying to Warzone2100/old-trac-import#1415 (comment:4):

May I recommend to add this MOD list to client game version(in the bottom right corner of the screen). So MODs will be not only in lobby list, but will be seen by player all the time.

Good idea, Terminator. I'll put it above the logo; easier to see that way.

7. Displays a list of mods above the Warzone logo.

Replying to Warzone2100/old-trac-import#1415 (comment:6):

About #4, if you want to discuss that, hiding it behind an unrelated label won't help there. I don't really like the approach of hardcoding more and more stuff.

Mmk, will make a separate ML post.

For real mods, some indication on the hosting screen would be nice, like "Autoloaded mods: abc, manually loaded: yzx. Note that all players need to have the same mods to join your game." Especially important for autoloaded mods people might forget about.

Leads to a second thing, distinction between autoloaded and other mods.

It wouldn't be too difficult to split the list into autoloaded and manually loaded, but it does add a bit of unnecessary complexity. The user is going to know which mods they've manually loaded, so they can easily tell for themself which mods are autoloaded. The game documentation for mods will mention autoload folders prominently, so someone consulting the documentation for why a mod is loaded will find it easily.

"Note that all players need to have the same mods to join your game." is a useful warning, though.

8. Lists mods in hosting screen, and warns hosts that all players need the same mods loaded.

Not sure I like the concept of autoloading itself, since it leads to what was mentioned above, if you want to host a game others can easily join, you need to quit, remove mod, restart, and that makes the whole thing somewhat less useful, in the long run, it's easier to just create the right shortcuts for mods.

Well, the user can choose which method they want to use.

Judging by the frequency of mod usage before and after the removal of autoload folders, it looks like they disagree with your definition of "easier". ;)

Adding the lists to the crashdump should be easy, but is also missing yet.

Don't know how to do that. Feel free to do it yourself.

-                      txt = _("Wrong data/mod detected by Host.");
+                      txt = _("You have an incompatible mod.");

Are you sure this can only happen when people use mods now?

Well, barring bugs, yes. The version checking code should take care of people using different game versions.

Newest patch: http://pastebin.ca/1754833

Zarel commented

(In [9290]) 2.3: Commit mod list patch #1415 (in preparation for 2.3 beta 8):

1. Adds a specific autoload folder, mods/autoload/. This is essential for a good mod community.
2. Searches the mods/ folder for global mods, as well as mods/global/. This is the first step in merging the three mod folders together, but I'm not planning on merging them completely in 2.3.
3. Constructs a list of loaded mods, and displays the list in the VersionString (including the list of games in the lobby).
4. Grays out games with incompatible mods in the NetPlay game list, and gives a warning in the tooltip: "Your loaded mods are incompatible with this game. (Check mods/autoload/?)"
5. Gives a warning when a game with an incompatible mod is clicked: "You have an incompatible mod."
6. Displays a list of mods above the Warzone logo.
7. Lists mods in hosting screen, and warns hosts that all players need the same mods loaded.

Closes #1415.

Zarel commented

(In [9291]) Commit mod list patch #1415:

1. Adds a specific autoload folder, mods/autoload/. This is essential for a good mod community.
2. Searches the mods/ folder for global mods, as well as mods/global/. This is the first step in merging the three mod folders together, but I'm not planning on merging them completely in 2.3.
3. Constructs a list of loaded mods, and displays the list in the VersionString (including the list of games in the lobby).
4. Grays out games with incompatible mods in the NetPlay game list, and gives a warning in the tooltip: "Your loaded mods are incompatible with this game. (Check mods/autoload/?)"
5. Gives a warning when a game with an incompatible mod is clicked: "You have an incompatible mod."
6. Displays a list of mods above the Warzone logo.
7. Lists mods in hosting screen, and warns hosts that all players need the same mods loaded.

Closes #1415.

Zarel commented

Committed in [9290]/[9291].

I had cybersphinx, Per, and Cyp_, voicing no objection, and Kreuvf and stiv voicing no opinion. Only objection appears to be the second comment on this ticket, a vague unsubstantiated anonymous "bad implementation and bug prone".

Zarel changed status from accepted to closed

Zarel set resolution to fixed

anonymous commented

Replying to Warzone2100/old-trac-import#1415 (comment:8):

Committed in [9290]/[9291].

I had cybersphinx, Per, and Cyp_, voicing no objection, and Kreuvf and stiv voicing no opinion. Only objection appears to be the second comment on this ticket, a vague unsubstantiated anonymous "bad implementation and bug prone".

If you would all actually play the damn game you would know what the problem is with this bad implementation.
Having a autoload approach without the proper error handling leads to more bugs.

If one side has a different version of the same mod they get kicked and host must restart game.
People need to know beforehand if it is the same mod(s) or not.
If they don't remove the mod and save the game then remove or add stuff from autoload in the next MP game they play and when that is over they try to load the save game again it crashes if the exact same mods are not there.

Don't give me this vague and unsubstantiated bullshit. You have no clue what your doing, refuse to think your approach is wrong and are only making the game more unstable as if it needs any more bugs.

Zarel commented

Replying to Warzone2100/old-trac-import#1415 (comment:10):

anonymous

Again, with whom do I have the honor of conversing?

If you would all actually play the damn game you would know what the problem is with this bad implementation.
Having a autoload approach without the proper error handling leads to more bugs.

The current error handling is sufficient. I play the game often enough - in fact, I'm midway through the Gamma campaign a playtest of campaign in 2.3-branch, in preparation for release.

If one side has a different version of the same mod they get kicked and host must restart game.
People need to know beforehand if it is the same mod(s) or not.
If they don't remove the mod and save the game then remove or add stuff from autoload in the next MP game they play and when that is over they try to load the save game again it crashes if the exact same mods are not there.

In other words, the exact same problems they would have if they loaded mods with the command-line?

I'm indeed planning on fixing this behavior, but this patch hardly makes it any worse.

Users did just fine with autoload folders back when they were still available. There are far more questions now about how to get mods to work now than there ever were when autoload folders were available.

Don't give me this vague and unsubstantiated bullshit. You have no clue what your doing, refuse to think your approach is wrong and are only making the game more unstable as if it needs any more bugs.

You just made five vague and unsubstantiated statements in two sentences. Need I say more?