Robust character class loading

[tegstewart]

Makes loading class data from the CharacterClass table more robust, to prevent invalid data and null exceptions.

Invalid data is replaced with defaults, and a specific warning is generated to point the user towards the problem. All strings are null checked and replaced with defaults.

In theory, new classes can be added with higher class ids, but I'm not sure how the client could create them, or how the rest of the server would deal with that, so milage may vary.

[NetDwarf]

Thank you for your contribution!

However I have to note that I do not see the use case yet. Silencing errors is often worse than letting it error out. For the null checks I do not see why someone would set null in any of these entries on purpose, but I'd let them. Same applies for 0. This is done intentionally and it should not be rectified preemptively.

[tegstewart]

Null is the string default, so running a replace which doesn't specify all the strings leads to null strings and null exceptions.

The problem is that errors and exceptions happen further down the line, with nothing to indicate CharacterClass data is the issue. That's what lead to me doing this; I was trying to figure out why Doppelganger code was throwing a null exception error, and it took me a while to realize the issue was with the table contents and not with updated code. Somebody not familiar with the code wouldn't ever figure that out.

In my mind, we need to be preventing exceptions further down the line. I'd prefer filtering out invalid data and giving warnings or errors, but we could load CharacterClasses in CharacterClassDB.Load() rather than when they're first accessed so it's obvious exceptions are the result of bad CharacterClass data.

[NetDwarf]

In my mind, we need to be preventing exceptions further down the line. I'd prefer filtering out invalid data and giving warnings or errors, but we could load CharacterClasses in CharacterClassDB.Load() rather than when they're first accessed so it's obvious exceptions are the result of bad CharacterClass data.

What was the exact error and how was it triggered?

[NetDwarf]

Would #450 be enough?

[tegstewart]

I did a bunch of testing, and the only error I've been able to reproduce is when there aren't any eligible races, and I didn't check the count on that before pulling from it in doppelganger code. There is some unexpected behaviour with stats, setting the primary stat out of range stop secondary and tertiary stats from being used, but it doesn't throw an exception or error.

I still lean towards aggressively vetting data as it's loaded rather than doing checks for it down the line, but that's a preference. Up to you either way.

[NetDwarf]

I did a bunch of testing, and the only error I've been able to reproduce is when there aren't any eligible races, and I didn't check the count on that before pulling from it in doppelganger code.

At worst this should be fixed in Doppelganger code. The data is actually fine (no null checks necessary).

I still lean towards aggressively vetting data as it's loaded rather than doing checks for it down the line, but that's a preference. Up to you either way.

I obviously lean more towards minimal checks/minimal code. Here this is especially noticeable, because this table is created with defaults and someone needs to actively change them to screw them up. So for me this seems more like an attempt to make it kidproof which is a very long endeavor and never really finished.

More generic tables which are regularly filled with contributed data that are not part of DOL might need some sanity checks to prevent common errors, but it should be kept to a minimum as well.

[tegstewart]

Sounds good. I'll close this, and remedy Doppelganger code next week.

Moving this weekend, so this week will be busy. :-)