Day 59 - Concerned

In which we explore our fears of code-database disagreement.

So here’s the issue: Back when it was available, there was a database backup bundled with the server source code. I’ve been working with that database so far, with great success. But there’s also a set of SQL scripts with the server source, which one would hope to be authoritative.

Tale of two Players

There’s not a lot of agreement between the existing database table (left) and the contents of the Player.sql script (right):

[account],                              [Account],                      
[accountID],                            [AccountID],                
[age],                                  [Age],                      
[alignment],                          | [Align],                
[ancestor],                           | [Anonymous],                    
[ancestorID],                         <
[bankGold],                             [BankGold],                 
[birthday],                             [Birthday],
                                      > [Blind],
                                      > [Bow],
                                      > [BreatheWater],
                                      > [CharClass], 
                                      > [CharClassFull],                
[charisma],                             [Charisma],                 
[classFullName],                      | [ClassID],              
[classType],                          <
[confRoom],                             [ConfRoom],                 
[constitution],                         [Constitution],             
[currentKarma],                       | [CurPos],
[dead],                               | [Dagger],
[deleteDate],                         | [DexAdd],
[dexterity],                            [Dexterity],
[dexterityAdd],                       <
[dirPointer],                           [DirPointer],
                                      > [Echo],
                                      > [Encumb],
[exp],                                  [Exp],
[facet],                              | [FeatherFall],
[fighterSpecialization],              | [FighterSpecial],
                                      > [Flail],
[floating],                             [Floating],
[gender],                               [gender],
                                      > [Halberd],
[hits],                                 [Hits],
[hitsAdjustment],                     <
[hitsDoctored],                       <
[hitsMax],                              [HitsMax],
                                      > [ImpImmortal],
[ImpLevel],                             [ImpLevel],
[intelligence],                         [Intelligence],
                                      > [IsDead],
                                      > [IsHidden],
                                      > [IsInvisible],
[land],                                 [Land],
[lastOnline],                         | [LastSave],
[level],                                [Level],
[lifetimeKarma],                      | [LHItemAttuned],
[lifetimeMarks],                      | [LHItemID],
                                      > [Mace],
                                      > [Magic],
[mana],                                 [Mana],
[manaAdjustment],                     <
[manaMax],                              [ManaMax],
[map],                                  [Map],
[name],                                 [Name], 
[notes],                              | [NightVision],
[numDeaths],                            [NumDeaths],
[numKills],                             [NumKills],
[playerID],                             [PlayerID],
[playersFlagged],                     | [Poisoned],
[playersKilled],                      | [protocol],
[pvpDeaths],                          <
[pvpKills],                           <
[race],                                 [Race], 
                                      > [Rapier],
                                      > [RHItemAttuned], 
                                      > [RHItemID],
[roundsPlayed],                         [RoundsPlayed],
                                      > [SavedHitsMax],
                                      > [SavedManaMax],
                                      > [ShowTitle],
                                      > [Shuriken],
                                      > [Staff],
[stamina],                              [Stamina],
[staminaAdjustment],                  <
[stamLeft],                             [StamLeft],
                                      > [StrAdd],
[strength],                             [Strength],
[strengthAdd],                        <
[stunned],                              [Stunned],
[UW_hitsAdjustment],                  | [Sword],
[UW_hitsMax],                         | [Thievery],
[UW_intestines],                      | [Threestaff],
[UW_liver],                           | [TimeOut],
[UW_lungs],                           | [trainedBow],
[UW_manaAdjustment],                  | [trainedDagger],
[UW_manaMax],                         | [trainedFlail],
[UW_staminaAdjustment],               | [trainedHalberd],
[UW_staminaMax],                      | [trainedMace],
[UW_stomach],                         | [trainedMagic],
[visualKey],                          | [trainedRapier],
                                      > [trainedShuriken],
                                      > [trainedStaff],
                                      > [trainedSword],
                                      > [trainedThievery],
                                      > [trainedThreestaff],
                                      > [trainedTwoHanded],
                                      > [trainedUnarmed],
                                      > [Twohanded],
                                      > [Unarmed],
[wisdom],                               [Wisdom],
[xCord],                                [XCord],
[yCord],                                [YCord],
[zCord]                               <

I have the feeling that the SQL script is either historical or prescriptive, and doesn’t match the existing code; a few searches should help.

Does the code refer to UW_intestines? Yes, it’s the source of UW_hasIntestines in the PC class, used in the Underworld:

if (chr.InUnderworld)
    if (chr.UW_hasIntestines) { chr.WriteToDisplay("You have your intestines."); }
    else { chr.WriteToDisplay("You do not have your intestines."); }

Does the code refer to trainedShuriken? Yes, but from the PlayerSkills table and not Player.

Does the code refer to fighterSpecialization or FighterSpecial? fighterSpecialization.

Conclusion: I should place my trust in DAL/DBPlayer.cs and the existing database, not Player.SQL. Good to know.

But… expect us?

And yet the server code is looking for an anonymous field in Player, which appears to actually exist in the PlayerSettings table. Mark it @bug, give a descriptive comment, and move on.


Wrapping legacy code in tests always leads to a quandary: what do we do with the bugs that we find in the existing code?

On the one hand, we don’t want to constantly stop to fix them; we’re trying to move forward, not work on the legacy code. We should write the test for the way it should work, tag it as an expected failure, and move on.

On the other hand, we shouldn’t leave portions of the code untested. We should either fix the bug or write the tests to expect the broken behavior.

Pragmatism: To maintain testing momentum, tag and move on. Every so often (let’s say five days), comb through and try to apply a bunch of the easy fixes at once. Any bug that can’t be fixed in less than a day gets marked Expected Behavior and becomes a high-priority Feature Request on the new code.

Short day today. More substantive entry tomorrow.