Day 80 - Code Dissatisfaction · 301 Days (redux)

Table of Contents

In which we question a lot of what we’ve written.

Test Every Day
#

Bless
#

The Bless potion just combined a few different effects and is pretty trivial (if time-consuming) to test:

features/player_effects.feature

696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715


Scenario: Bless potion has desired effect
  Given I use the "minimal" database as-is
  And I add player and character "TestBP01"
  And I set the character's "dexterity" stat to "10"
  And I set the character's "constitution" stat to "8"
  And I create an "Bless" potion of amount "0" and duration "6"
  And I put the item in the character's left hand
  And I start the server and play
  When I open and drink "bottle"
  And I have a "dexterity" stat of "10"
  And I have a temporary "dexterity" stat of "1"
  And I have a "constitution" stat of "8"
  And I have a temporary "constitution" stat of "1"
  And I have a shielding of "1"
  And I rest
  And I have a "dexterity" stat of "10"
  And I have a temporary "dexterity" stat of "0"
  And I have a "constitution" stat of "8"
  And I have a temporary "constitution" stat of "0"
  And I have a shielding of "0"

Wizard_Eye
#

This is a lot more interesting. From the server code, in CreateCharacterEffect:

DragonsSpine/GameSystems/Effects/Effect.cs

GitLab

831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855


  case EffectType.Wizard_Eye:
    target.MyClone = target.CloneCharacter();
    target.MyClone.IsPC = false;
    Effect.CreateCharacterEffect(EffectType.Wizard_Eye, 0, target.MyClone, duration, target);
    Effect.CreateCharacterEffect(EffectType.Hide_In_Shadows, 0, target, -1, target);
    target.MyClone.CurrentCell = target.CurrentCell;
    target.MyClone.AddToWorld();
    if (target.BaseProfession == Character.ClassType.Thief)
      target.Name = "rat";
    else
      target.Name = "toad";
    Character.SetCharacterVisualKey(target);
    target.RightHand = null;
    target.LeftHand = null;
    target.Hits = 3;
    target.HitsMax = 3;
    target.HitsAdjustment = 0;
    target.Stamina = 3;
    target.StaminaMax = 3;
    target.StaminaAdjustment = 0;
    target.Mana = 0;
    target.ManaMax = 0;
    target.ManaAdjustment = 0;
    target.animal = true;
    break;

So it creates an NPC clone of the target, puts the same wizard eye effect on the clone, hides the target in shadows indefinitely, brings the clone into the world at the same location as the target, and changes the target to a toad (or rat if they are a thief).

That’s a bunch of stuff to test; let’s try something simple first:

features/player_effects.feature

GitLab

749
750
751
752
753
754
755
756
757
758
759


Scenario: Wizard Eye potion results in OOBE
  Given I use the "minimal" database as-is
  And I add player and character "TestHS01"
  And I create an "Wizard_Eye" potion of amount "0" and duration "2"
  And I put the item in the character's left hand
  And I start the server and play
  When I open and drink "bottle"
  Then I saw myself
  And I see my doppelganger in a trance
  And I rest
  And I did not see myself

features/steps/character_steps.rb

GitLab

396
397
398
399
400
401
402
403
404
405
406
407
408
409


Then(/^I saw myself$/) do
  expect(@last_resp).to match(/\b#{@user[:char_name]}\b/)
end

Then(/^I did not see myself$/) do
  expect(@last_resp).to_not match(/\b#{@user[:char_name]}\b/)
end

Then(/^I see my doppelganger in a trance$/) do
  # The gsub is in case this text is split between lines
  resp = telnet_command(@connection, "look at #{@user[:char_name]}", / ->/).gsub(/\s+/, ' ')
  waited_for = /\b#{@user[:char_name]} is in a trance./i
  expect(resp).to match(waited_for)
end

It works, although I’m starting to get uncomfortable with our lack of parsing on the output: there’s no distinction between the map, the character list, flavor text, and stats bar. False positives are definitely possible where we mistake text in one for text in another.

While we contemplate that, let’s go one step further:

features/player_effects.feature

GitLab

762
763
764
765
766
767
768
769
770
771
772
773


Scenario: Wizard Eye potion effect visible by others
  Given I use the "minimal" database as-is
  And I add player and character "TestHS01"
  And I create a "Wizard_Eye" potion of amount "0" and duration "50"
  And I put the item in the character's left hand
  And I start the server and play
  When I open and drink "bottle"
  And I add player and character "TestHS02"
  And I log on as "TestHS02"
  And I enter the game
  Then I saw a "toad"
  And I see "TestHS01_name" in a trance

And something else I’m not comfortable with: the way we handle multiple characters being used in a test. Interesting player interaction is going to take a lot of work; so let’s stop there for now.

Tackle a TODO every other day
#

Empty Hands
#

features/config_file.feature

GitLab

266
267
268
269


## RequireMakeRecallReagent
# TODO - empty hands between tests, use database as-is
Scenario: RequireMakeRecallReagent False, thief can make recall ring from nothing
  Given I use the "minimal" database

So I add a hook and tag the tests:

features/lib/hooks.rb

GitLab

40
41
42


After('@clear_player_hands') do
  clear_player_hands
end

features/lib/db/player.rb

GitLab

166
167
168
169
170
171
172
173
174


def clear_player_hands
  client = connect_to_db(@server_database)
  query = "UPDATE [dbo].[PlayerHeld] SET \
  itemID = 0, attunedID = 0, special = ''"
  debug_msg query
  result = client.execute(query)
  affected_rows = result.do
  debug_msg "Cleared #{affected_rows} player hands"
end

But then I realized that I already had a hook for clearing all of the player stuff in the database, and it’d be far simpler to just use that after every test than worry about whether I’d altered hands or effects or inventory or whatever.

features/lib/hooks.rb

GitLab

50
51
52
53
54


After('@db_cleanup') do
  delete_all_test_accounts
  delete_all_test_players
  clear_log
end

New Player
#

Rubocop’s been providing me with lots of additional TODOs. It didn’t like my db_insert_player method, and neither did I:

features/lib/db/player.rb

GitLab

19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51


def db_insert_player(db_hash)
  # anything in the hash overrides defaults
  player = DEFAULT_PLAYER.merge(db_hash)
  client = connect_to_db(@server_database)
  query = "INSERT INTO [#{@server_database}].[dbo].[Player] ([notes], [accountID], [account], \
  [name], [gender], [race], [classFullName], [classType], [visualKey], [alignment], [confRoom], \
  [impLevel], \
  [ancestor], [ancestorID], [facet], [land], [map], [xCord], [yCord], [zCord], [dirPointer], \
  [stunned], [floating], [dead], [fighterSpecialization], [level], [exp], [hits], [hitsMax], \
  [hitsAdjustment], [hitsDoctored], [stamina], [stamLeft], [staminaAdjustment], [mana], \
  [manaMax], [manaAdjustment], [age], [roundsPlayed], [numKills], [numDeaths], [bankGold], \
  [strength], [dexterity], [intelligence], [wisdom], [constitution], [charisma], [strengthAdd], \
  [dexterityAdd], [birthday], [lastOnline], [deleteDate], [currentKarma], [lifetimeKarma], \
  [lifetimeMarks], [pvpKills], [pvpDeaths], [playersKilled], [playersFlagged], [UW_hitsMax], \
  [UW_hitsAdjustment], [UW_staminaMax], [UW_staminaAdjustment], [UW_manaMax], \
  [UW_manaAdjustment], [UW_intestines], [UW_liver], [UW_lungs], [UW_stomach]) \
  VALUES (N'', #{player[:accountID]}, \
  N'#{player[:account]}', \
  N'#{player[:name]}', \
  1, N'Illyria', N'Fighter', \
  N'Fighter', N'male_fighter_pc_brown', 1, 0, #{player[:impLevel]}, 0, 0, 0, 0, 0, 42, 27, 0, \
  N'v', 0, 3, 0, N'None', 4, 4996, 36, 36, 0, 0, 10, 10, 0, 0, 0, 0, 806, 806, 8, \
  0, 0, 17, 18, 10, 17, 8, 18, 1, 1, CAST(0x0000A4FC0001D163 AS DateTime), \
  CAST(0x0000A4FC001A5F6A AS DateTime), CAST(0x0000979200000000 AS DateTime), \
  0, 0, 0, 0, 0, N'', N'', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)"
  result = client.execute(query)
  affected_rows = result.do
  fail "Unable to insert player!" if 1 != affected_rows
  add_spell_slots_for_player(get_player_id(player[:name]))
  add_hand_slots_for_player(get_player_id(player[:name]))
  add_effect_slots_for_player(get_player_id(player[:name]))
  add_skills_for_player(get_player_id(player[:name]))
end

So we move all of the defaults into the DEFAULT_PLAYER hash:

features/lib/db/player.rb

GitLab

 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20


DEFAULT_PLAYER = {
  accountID: 0, account: 'no_name', name: 'no_name', gender: 1, race: 'Illyria',
  classFullName: 'Fighter', classType: 'Fighter', visualKey: 'male_fighter_pc_brown', alignment: 1,
  confRoom: 0, impLevel: 0, ancestor: 0, ancestorID: 0, facet: 0, land: 0, map: 0, xCord: 42,
  yCord: 27, zCord: 0, dirPointer: 'v', stunned: 0, floating: 3, dead: 0,
  fighterSpecialization: 'None', level: 4, exp: 4996, hits: 36, hitsMax: 36, hitsAdjustment: 0,
  hitsDoctored: 0, stamina: 10, stamLeft: 10, staminaAdjustment: 0, mana: 0, manaMax: 0,
  manaAdjustment: 0, age: 806, roundsPlayed: 806, numKills: 8, numDeaths: 0, bankGold: 0,
  strength: 17, dexterity: 18, intelligence: 10, wisdom: 17, constitution: 8, charisma: 18,
  strengthAdd: 1, dexterityAdd: 1, birthday: :last_week, lastOnline: :yesterday,
  deleteDate: :last_year, currentKarma: 0, lifetimeKarma: 0, lifetimeMarks: 0, pvpKills: 0,
  pvpDeaths: 0, playersKilled: '', playersFlagged: '', UW_hitsMax: 0, UW_hitsAdjustment: 0,
  UW_staminaMax: 0, UW_staminaAdjustment: 0, UW_manaMax: 0, UW_manaAdjustment: 0,
  UW_intestines: 0, UW_liver: 0, UW_lungs: 0, UW_stomach: 0
}.freeze

And then we can make db_insert_player nice and Rubocop-friendly:

features/lib/db/player.rb

GitLab

37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52


def db_insert_player(db_hash)
  debug_msg player = DEFAULT_PLAYER.merge(db_hash) # anything in the hash overrides defaults
  client = connect_to_db(@server_database)
  debug_msg query = db_insert_from_hash('Player', player)
  result = client.execute(query)
  affected_rows = result.do
  fail 'Unable to insert player!' if 1 != affected_rows
  add_misc_to_player(get_player_id(player[:name]))
end

def add_misc_to_player(player_id)
  add_spell_slots_for_player(player_id)
  add_hand_slots_for_player(player_id)
  add_effect_slots_for_player(player_id)
  add_skills_for_player(player_id)
end

Fix a Bug in the Legacy Code Every Fifth Day
#

features/player_effects.feature

GitLab

57
58
59
60
61
62
63
64
65
66
67
68


## Ale

# bug - No indication of the burp is sent to the player themselves.
@bug
Scenario: Ale effect gives the desired output - same player, beginning
  Given I use the "minimal" database as-is
  And I add player and character "TestAle01"
  And I give the character an "ale" effect of "999" for "2" turns
  And the server is started
  When I log on as "TestAle01"
  And I enter the game
  And I saw a message "You burp loudly."

I can certainly add a burp to the server code (and I did, briefly); but that’s not where it belongs, is it?

EntireDB-minimal.sql

GitLab

603
604
605
606
607


CREATE TABLE [dbo].[CatalogItem](
  [catalogID] [int] IDENTITY(1,1) NOT NULL,
  [notes] [nvarchar](255) NULL,
  [combatAdds] [int] NOT NULL,
  [itemID] [int] NOT NULL,

…

EntireDB-minimal.sql

GitLab

649
650
651
652


  [drinkDesc] [varchar](255) NOT NULL,
  [fluidDesc] [varchar](255) NOT NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO

The drinkDesc is what’s intended to be shown to the player themselves when an item is drunk. So let’s fix the test, not the server:

features/player_effects.feature

GitLab

59
60
61
62
63
64
65
66
67


Scenario: Ale effect gives the desired output - same player, beginning
  Given I use the "minimal" database as-is
  And I add player and character "TestAle01"
  And I create an "Ale" potion of amount "0" and duration "2"
  And I give the item a "drinkDesc" of "BURP!"
  And I put the item in the character's left hand
  And I start the server and play
  When I open and drink "bottle"
  And I saw a message "BURP!"

features/steps/character_steps.rb

GitLab

54
55
56
57


Given(/^I give the item (a|an) "([^"]*)" of "([^"]*)"$/) do |_aan, field, value|
  new_hash = Hash[field.to_sym, value]
  @item_id = create_item_from_blank({ itemID: @item_id }, new_hash)  
end

I use my nifty create_item_from_blank method, using the existing item as the “blank”, to make a new item with a changed field.

That works fine, but when I fix the player-in-the-same-cell test:

features/player_effects.feature

GitLab

80
81
82
83
84
85
86
87
88
89
90
91
92
93


Scenario: Ale effect gives the desired output - other player in same cell
  Given I use the "minimal" database as-is
  And I add player and character "TestAle01"
  And I create an "Ale" potion of amount "0" and duration "2"
  And I give the item a "drinkDesc" of "BURP!"
  And I put the item in the character's left hand
  And I add player and character "TestAle02"
  And the server is started
  When I log on as "TestAle02"
  And I enter the game
  And I rest
  And I cause "TestAle01" to log on and enter the game
  And I cause the secondary player to "open bottle; drink bottle"
  Then within "4" turns I see the message "TestAle01_name burps loudly."

I still don’t see it. Maybe there really is a bug to fix on the server side!

The effect code calls uses SendToAllInSight, so let’s add a bunch of debug to that:

DragonsSpine/GameObjects/GameLiving/Character/Character.cs

GitLab

2883
2884
2885
2886
2887
2888
2889
2890
2891
2892
2893
2894
2895
2896
2897
2898
2899
2900
2901
2902
2903
2904
2905
2906
2907
2908
2909
2910
2911
2912
2913
2914
2915
2916
2917
2918
2919
2920
2921
2922
2923
2924
2925
2926
2927
2928
2929
2930
2931
2932


public void SendToAllInSight(string text) // send text to everyone in sight, except this character
{
  int bitcount = 0;
  Cell curCell = null;

  for (int ypos = -3; ypos <= 3; ypos++)
  {
    for (int xpos = -3; xpos <= 3; xpos++)
    {
      Utils.Log("SendToAllInSight from " + this.Name + " considering relative cell " + xpos + ", " + ypos + "(bitcount " + bitcount + ")", Utils.LogType.Unknown);
      if (Cell.CellRequestInBounds(this.CurrentCell, xpos, ypos))
      {
        Utils.Log("SendToAllInSight from " + this.Name + " considers relative cell " + xpos + ", " + ypos + " in bounds.", Utils.LogType.Unknown);
        if (this.CurrentCell.visCells[bitcount]) // check the PC list, and char list of the cell
        {
          Utils.Log("SendToAllInSight from " + this.Name + " considers relative cell " + xpos + ", " + ypos + " visible?", Utils.LogType.Unknown);
          curCell = Cell.GetCell(this.FacetID, this.LandID, this.MapID, this.X + xpos, this.Y + ypos, this.Z);

          if (curCell != null)
          {
            Utils.Log("SendToAllInSight from " + this.Name + " considers relative cell " + xpos + ", " + ypos + " non-null?", Utils.LogType.Unknown);
            foreach (Character chr in curCell.Characters) // search for the character in the charlist of the cell
            {
              Utils.Log("SendToAllInSight from " + this.Name + " considering " + chr.Name + ".", Utils.LogType.Unknown);
              if (chr != this && chr.IsPC && this.IsPC) // players sending messages to other players
              {
                Utils.Log("SendToAllInSight from " + this.Name + " considering " + chr.Name + " further.", Utils.LogType.Unknown);
                if (Array.IndexOf(chr.ignoreList, this.PlayerID) == -1) // ignore list
                {
                  Utils.Log("SendToAllInSight from " + this.Name + " sending \"" + text + "\" to " + chr.Name + ".", Utils.LogType.Unknown);
                  if (chr.filterProfanity) // profanity filter
                  {
                    text = Conference.FilterProfanity(text);
                  }
                  Utils.Log("SendToAllInSight from " + this.Name + " sending (filtered) \"" + text + "\" to " + chr.Name + ".", Utils.LogType.Unknown);
                  chr.WriteToDisplay(text);
                }
              }
              else if (chr != this && chr.IsPC && !this.IsPC) // non players sending messages to other players
              {
                chr.WriteToDisplay(text);
              }
            }//end foreach
          }
        }
        bitcount++;
      }
    }
  }
}

After enabling all of the debug, we can see that the bitcount index into the visCells array is behaving oddly.

12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -3, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -2, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -1, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell 0, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell 1, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell 2, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell 3, -3(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -3, -2(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -2, -2(bitcount 0)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considers relative cell -2, -2 in bounds.
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell -1, -2(bitcount 1)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considers relative cell -1, -2 in bounds.
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considering relative cell 0, -2(bitcount 2)
12/23/2016 10:09:13 PM: {Unknown} SendToAllInSight from TestAle02_name considers relative cell 0, -2 in bounds.

Another look at the server code, and we see why: bitcount is only incremented for cells that are in bounds. Since our test map is quite small, and players are spawning near the edge, a portion of their immediate area is out of bounds.

I could tweak the server code to behave the way I think it was intended to, but I feel certain that there are probably other edge-of-the-map issues waiting for me. So instead I’ll expand the test map so the players are always safely away from the edge.

maps/test01.txt

GitLab

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15


<z>0</z> <x>38</x> <y>23</y> <f>light</f> <n>Test01 Surface</n> outdoor=true
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
WWWWWW. . . . . . . . . . . WWWWWW
WWWWWW. . . . . . . . . a0. WWWWWW
WWWWWW. . . . . . . . . . . WWWWWW
WWWWWW. . . . . . . . . a1. WWWWWW
WWWWWW. . . . . . . . . . . WWWWWW
WWWWWW. . . . . . . . . gg. WWWWWW
WWWWWW. . . . . . . . . . . WWWWWW
WWWWWW. . . . . . . . . . . WWWWWW
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW

12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering relative cell -1, 0(bitcount 23)
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell -1, 0 in bounds.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell -1, 0 visible?
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell -1, 0 non-null?
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering relative cell 0, 0(bitcount 24)
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 0, 0 in bounds.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 0, 0 visible?
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 0, 0 non-null?
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering TestAle02_name.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering TestAle02_name further.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name sending "TestAle01_name burps loudly." to TestAle02_name.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name sending (filtered) "TestAle01_name burps loudly." to TestAle02_name.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering TestAle01_name.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considering relative cell 1, 0(bitcount 25)
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 1, 0 in bounds.
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 1, 0 visible?
12/24/2016 3:36:38 PM: {Unknown} SendToAllInSight from TestAle01_name considers relative cell 1, 0 non-null?

Much better.

124 scenarios (124 passed)
1241 steps (1241 passed)
50m36.933s

Interesting Stuff
#

Day 80 code - tests

Day 80 code - server

Test Every Day #

Bless #

Wizard_Eye #

Tackle a TODO every other day #

Empty Hands #

New Player #

Fix a Bug in the Legacy Code Every Fifth Day #

Interesting Stuff #

Test Every Day
#

Bless
#

Wizard_Eye
#

Tackle a TODO every other day
#

Empty Hands
#

New Player
#

Fix a Bug in the Legacy Code Every Fifth Day
#

Interesting Stuff
#