Disappearing Ground Units

[SpikeTheHobbitMage]

The one thing that seems clear about this issue is that you need more information to track down the root cause.  Would you consider adding error logging to the next release so that you would have a concise and accurate report on what occurred around the time of the error? 

Part of the problem in a bug of this nature is human perception and rather coarse capabilities, a key clue might easily go unnoticed or be forgotten, the granularity of the snapshots you are able to look at (before and after databases) is evidently insufficient in this case and could possibly be likewise lacking in tracking later issues.

I'm retired so not very active but I spent some time talking to a few friends who are still on the job and there was a unanimous consensus on the need for logging in cases like these.

Hope this helps

So much this.  If nothing else, logging the bad rows would at least tell you why they were rejected.

[Erik L]

This is what I use to log everything in my programs. Yes I know there are logger extensions/addins/whatevers, but this works for me.

Code: [Select]

       public static void WriteLog(string message, LogFile logType, LogLevel level, string moduleName = "", string gameName = "", string raceName = "")
        {
            string logFile;
            string header;

            DateTimeFormatInfo dtfi = new CultureInfo(currCultureInfo.ToString(), false).DateTimeFormat;
            dtfi.LongTimePattern = "HH:mm:ss";

            header = "[" + DateTime.Today.ToLongDateString() + " " + DateTime.Now.ToLongTimeString() + "] - ";

            switch (level)
            {
                case LogLevel.errorLevel:
                    header = header + "[ERROR]: " + moduleName;
                    break;
                case LogLevel.debugLevel:
                    header = header + "[DEBUG]: " + moduleName;
                    break;
                default:
                    header = header + "[INFO]: " + moduleName;
                    break;
            }
            message = header + message + "\r\n";

            switch (logType)
            {
                case LogFile.errorLog:
                    logFile = LogPath + @"\Error.log";
                    break;
                case LogFile.debugLog:
                    logFile = LogPath + @"\Debug" + DateTime.Today.ToString("yyyymmdd") + ".log";
                    break;
                case LogFile.gameLog:
                    logFile = LogPath + @"Game-" + gameName + ".log";
                    break;
                case LogFile.raceLog:
                    logFile = LogPath + @"Race-" + raceName + ".log";
                    break;
                default:
                    logFile = LogPath + @"\General.log";
                    break;

            }
            if (!File.Exists(logFile))
            {
                using (StreamWriter file = new StreamWriter(logFile))
                {
                    file.Write(message);
                }
            }
            else
            {
                using (StreamWriter file = new StreamWriter(logFile, true))
                {
                    file.Write(message);
                }
            }

        }

[RedDagger]

Then I shall endeavor to give good advice.   

You will need to fully suppress any row-level exceptions unless you intend to abort the whole save, because the using clause on the transaction will rollback if it sees one.   Returning from inside the loop has the same effect.   Maybe log those errors to a text file instead?  Remember to open in append mode. Edit: Actually, just remove the return after showing the popup so the save can continue.

Another issue is that if a non-recoverable error does occur or Aurora crashes during the save, only the current table gets rolled back.   That is guaranteed to leave the database in an inconsistent state with the current setup.   The solution to that problem is to start the transaction in the function that calls the save functions.   Just pass the connection in as an argument so they can all share it.   This has the added bonus of speeding up saves by a factor of about 7x.

Instead of passing the connection already opened as a parameter in each function, Steve can put it in a Singleton class.  It might be less work refactoring.

[Omnivore]

Then I shall endeavor to give good advice.   

You will need to fully suppress any row-level exceptions unless you intend to abort the whole save, because the using clause on the transaction will rollback if it sees one.   Returning from inside the loop has the same effect.   Maybe log those errors to a text file instead?  Remember to open in append mode. Edit: Actually, just remove the return after showing the popup so the save can continue.

Another issue is that if a non-recoverable error does occur or Aurora crashes during the save, only the current table gets rolled back.   That is guaranteed to leave the database in an inconsistent state with the current setup.   The solution to that problem is to start the transaction in the function that calls the save functions.   Just pass the connection in as an argument so they can all share it.   This has the added bonus of speeding up saves by a factor of about 7x.

Instead of passing the connection already opened as a parameter in each function, Steve can put it in a Singleton class.  It might be less work refactoring.

Once I got past my instinctive NO GLOBALS ARE EVIL reaction... ok I'm still not over it, but if it was deemed absolutely necessary you'd have to make it compatible with using statements and make sure it threw exceptions if accessed outside of such a scope, otherwise asking for trouble.

PS: Addressing all the concerns involved to make it robust would probably take more time and effort than passing the connection.

[Migi]

As a non-programmer, I have only one probably unhelpful suggestion:
Did you sanitize your database inputs?
Is this caused by people mis-naming their drop pod troops as drop table troops?
(relevant xkcd)

[SpikeTheHobbitMage]

Instead of passing the connection already opened as a parameter in each function, Steve can put it in a Singleton class.  It might be less work refactoring.

That seems to me to be overkill for something that could just as easily be tied to the game-state or some other long lived object if he wants to keep the connection open across saves.

Once I got past my instinctive NO GLOBALS ARE EVIL reaction... ok I'm still not over it, but if it was deemed absolutely necessary you'd have to make it compatible with using statements and make sure it threw exceptions if accessed outside of such a scope, otherwise asking for trouble.

Globals are evil and also unnecessary.  There is also no need to use a 'using' statement with a persistent connection as it will only be opened once and is crash-safe.  The transactions and prepared statements are the parts that need reliable cleanup.

As a non-programmer, I have only one probably unhelpful suggestion:
Did you sanitize your database inputs?
Is this caused by people mis-naming their drop pod troops as drop table troops?
(relevant xkcd)

Based on his sample code, Steve has learned the Sacred Art of the Prepared Statement.  Input sanitization is assured.

[se5a]

Globals are *not* evil, and static and Singleton use are not even as bad as globals.
Globals should be used with great care, yes, but there are times when you can do far more damage to your codebase by avoiding a global than you would by using it. go watch some of Jon Blow's rants on this sort of thing.
Sorry for the off topic, but this is part of a disease that has been spreading through the programming community for a couple of decades, it's why we have to deal with such smegty slow software on a day to day basis.

[TMaekler]

Don't want to create a side discussion; but seeing the potential problems with saving, I was wondering if using your own save format might solve the issues. Using a DB now derives from using it in the past because easier to handle when not all data is in RAM; but now it is... . So might be worth while to think about this.

[Kelewan]

I have not triggered this bug jet, but i have followed this and the other bug reports.
Reading through the last paged i came to the conclusion that the function that saves
the ground forces is not the root of the problem.

Steve uses a transaction  around the inserts. So all or none of the ground forces  should be in the database.
Most reports say that some but not all ground forces are missing and there are no error messages shown.
This leads to my conclusions

• that there is no error inserting the ground forces in the database
• that the missing ground forces where already missing in the data structure

 

So how do the ground forces go missing? What do we know?

• in most cases it was a group of ground forces: e.g. all STOs, all recent created troops, all initial created troops
• the action/actions didn't remove the troops from the display immediately, because nobody clearly remembers what they have done with the troops before
• the action/actions are not that common or we would have more reports

Is there a place where the ground force data structure is copied and the data structure used to display diverges from the one that is used to save?

[se5a]

Is there a place where the ground force data structure is copied and the data structure used to display diverges from the one that is used to save?

There's a bunch of places where the UI will not refresh unless the window is closed and reopened again, or the refresh button is clicked, this may explain the above.

[Steve Walmsley (OP)]

I have not triggered this bug jet, but i have followed this and the other bug reports.
Reading through the last paged i came to the conclusion that the function that saves
the ground forces is not the root of the problem.

Steve uses a transaction  around the inserts. So all or none of the ground forces  should be in the database.
Most reports say that some but not all ground forces are missing and there are no error messages shown.
This leads to my conclusions

• that there is no error inserting the ground forces in the database
• that the missing ground forces where already missing in the data structure

 

So how do the ground forces go missing? What do we know?

• in most cases it was a group of ground forces: e.g. all STOs, all recent created troops, all initial created troops
• the action/actions didn't remove the troops from the display immediately, because nobody clearly remembers what they have done with the troops before
• the action/actions are not that common or we would have more reports

Is there a place where the ground force data structure is copied and the data structure used to display diverges from the one that is used to save?

It does sound like some people are only losing some of their ground forces, so if that is the case you are correct that a save error would not cause that.

All ground formations are saved in a single collection within the Game object, which is loaded from the database. This acts in effect as a ground forces table. Then if I need to populate the OOB for a particular race for example, I will use LINQ to select the ground forces of a that race into a new list, but this is just a list of references to the master list. Only one object for each formation exists within the program. Objects are added to or removed from the master list when a formation is created or destroyed. That master list is then iterated for the save.

Based on this thread, I think I need to improve the save code anyway, but I agree that may not actually be the cause of the bug.

[Steve Walmsley (OP)]

Don't want to create a side discussion; but seeing the potential problems with saving, I was wondering if using your own save format might solve the issues. Using a DB now derives from using it in the past because easier to handle when not all data is in RAM; but now it is... . So might be worth while to think about this.

The database is also really useful for viewing and modifying data outside the program.

[TMaekler]

Don't want to create a side discussion; but seeing the potential problems with saving, I was wondering if using your own save format might solve the issues. Using a DB now derives from using it in the past because easier to handle when not all data is in RAM; but now it is... . So might be worth while to think about this.

The database is also really useful for viewing and modifying data outside the program.

I know. Did that myself as I had written you with the 1.7. to 1.8 DB - worked fine by the way. No errors since 

[Froggiest1982]

Wouldn't launch the game without privileged access (admin) cause some issues when writing/overwriting the database? I don't know if relevant...

it's so random and not everybody is affected so there must be something in common. I never had this issue yet (thank God) for instance so I really don't know, all I know is I got paranoid so I save every 30 minutes, close all windows prior saving and then run an interval and save again. I know I said that before, but the more I read here the more I get confused.

[Steve Walmsley (OP)]

BTW can I just clarify if the units disappear after save, or after the game is reloaded?