Recent - deadlock loading saved game

[jamespetts]

In the latest 11.x build, I am getting a deadlock on loading the saved game from the Bridgewater-Brunel server. This occurs with all fairly recent saves from the Bridgewater-Brunel server game, including ones from some time ago that are already uploaded (including this). It appears to occur quite early in the loading process. This is a new error, and does not occur in RC 11.9007.

Edit: Further testing shows that the problem occurs in loading the city rules.

Edit 2: The issue seems to be that, in the line:

Code Select Expand


file->rdwr_long(count);


the number "count" is read as being 50593792, which makes this process take an unfeasibly long time. This is clearly a dubious number. The question is how it gets there...

[Junna]

I do not have this problem on what I compiled from your 11.x branch, which I downed from git last night.

[jamespetts]

Now fixed: this code was responsible:

Code Select Expand


file->rdwr_long(cluster_factor);
file->rdwr_long(bridge_success_percentage);


This was added without reversioning to code that was not exclusively called in network games.

[isidoro]

I'm glad you managed to solve it.  But, strictly speaking, that is not a deadlock in computer science... 

[jamespetts]

No, I realised that it wasn't an actual deadlock later - but MSVC++ seemed to think that it was at one point. Multi-threading might have confused it.

[neroden]

Now fixed: this code was responsible:

Code Select Expand


file->rdwr_long(cluster_factor);
file->rdwr_long(bridge_success_percentage);


This was added without reversioning to code that was not exclusively called in network games.

It's not exclusively called in network games?

Um, big yuck?  That means I have to go through and rewrite the system for loading city rules.  And probably electricity, too.

The network game architecture is BROKEN.  It requires copying a huge number of pak settings which should not be present in a saved game at all.  (It should, instead, be copying the *pak config file* over the network.)

I recently figured out how to override this broken, defective-by-design system for the simuconf.tab file.  But apparently I have to redo this work for cityrules.tab, and presumably also privatecar.tab and electricity.tab.  This is disgusting.  This is not how this should have been designed.

I strongly advise that someone rewrite the network code to pass the pak configuration settings.  We need to stop saving this garbage in saved games.

---
I have just fixed a whole bunch of other bugs as well as adding a couple of "prettification" features.  Some of these bugs were practically impossible to detect without loading an *old* saved game with *new* pak rules.  It's something I want to do constantly during development.

[jamespetts]

The idea of transmitting the settings file itself is an interesting one - I don't know whether this was considered at the time by the authors of the Standard networking code. I'm not sure that it's quite accurate to describe it as broken, since it works as intended and does not have any game-breaking side effects. It might be said to be sub-optimal, although I can see arguments either way (one might want to retain existing settings so as to keep the balance of an existing saved game, but update to a newer version of a pakset, with different default settings, in order to use new vehicles, etc.). Actually, the best solution, I think, is the one that you have just implemented, with an optional over-ride, which gives players the best of both worlds. Is this code mature enough to go into the next release yet, do you think, or do you need to rework it first to deal with the fact that some of these settings are saved even in non-network games? (Or have you now done this? Some of your latest commits suggest that you have, but it would be helpful to be sure.)

Thank you very much for your recent bug fixes and cosmetic works - they are appreciated, as ever.

[neroden]

The idea of transmitting the settings file itself is an interesting one - I don't know whether this was considered at the time by the authors of the Standard networking code. I'm not sure that it's quite accurate to describe it as broken, since it works as intended and does not have any game-breaking side effects.

Programmer-breaking side effects, instead.    This design choice has caused me days of headaches.

It might be said to be sub-optimal, although I can see arguments either way (one might want to retain existing settings so as to keep the balance of an existing saved game, but update to a newer version of a pakset, with different default settings, in order to use new vehicles, etc.). Actually, the best solution, I think, is the one that you have just implemented, with an optional over-ride, which gives players the best of both worlds. Is this code mature enough to go into the next release yet, do you think, or do you need to rework it first to deal with the fact that some of these settings are saved even in non-network games? (Or have you now done this? Some of your latest commits suggest that you have, but it would be helpful to be sure.)

The override code is now definitely mature enough to go into the next release.  People don't have to turn it on, after all.  I suspect that it still has bugs (there are some really subtle things in the way settings are loaded)  but it's better than not having it.  Since I will generally leave it on, I should be able to spot any further bugs in it over time.

[jamespetts]

Already ahead of you on this one - the override code is in 11.0. It is a sensible idea that will hopefully address what was formerly much player confusion.