Feature: ORM Driven Character Positions for Issue #166 #184

fantoms · 2017-03-30T02:10:18Z

This huge PR is related to Issue #166. The code has been changed to allow more then one position type. A class was added for Character position types that match the requirements listed in the issue. A secondary Position Type class was added to match the client, provided by @LtRipley36706.

To work with the ORM code introduced during the developer meeting, I've attempted to copy the pseudo 1:1 mapping into CharacterPosition.cs. In order to allow for more then one position, the Character.Position member was changed to dictionary containing a list of character positions by type. Player.Position was changed to Player.PhysicalPosition and when the server decides to save the positions to the database, the data is saved through the ORM object in CharacterPosition.cs.

There were a few things left to do, that were not accomplished for Issue #166:

Hook up /lifestone and /ls commands

ChangeLog:

Changed the Player.Position to Player.PhysicalPosition to match the Developer meeting video and Issue Give Characters a set of positions #166.
Created CharacterPostion and CharacterPositionType classes to store the Database types requested in Issue Give Characters a set of positions #166.
Added CharacterPostion ORM logic.
Built out helper extension functions to serve the StartingLocation, until we pull from the database. Also included an Invalid location as a default (0).
Changed the character_positions table and Added positionType column.
Set the primary key to include positionType column.
Added positionType to required Critera for character_positions.
Added CharacterPositionInsert and CharacterPositionUpdate to the Character Database.
Created CharacterPositionInsert and CharacterPositionUpdate prepared SQL statements by utilizing the ConstructStatement function.
Added the required positionType clause to the CharacterPositionSelect prepared statement.
Added a position update statement for updating the player position.
Reworked GetPosition to include the CharacterPositionType.
Added function for getting a CharacterPostion from the database, requires a character Id and CharacterPositionType.
Added logic to get location function to insert a location into the database, if not present.
Reworked the UpdateCharacter and Update functions to save positions.
Added functions for inserting new character positions on character creation.
Added functions for saving/loading the character positions from the database on joining the World.
Attempted to get log4net working properly and moved some code to logging.
Added DBDEBUG project constant for debugging the database calls.
Changed the CharacterPositionSelect prepared statement to a ConstructStatement.

GitLog:

Changed position to phsysical/logical and added logical positions
Updated comments on CharacterPositionType
Added the sql for altering the positions table, to add the positionType
Added ripleys Position Types
Added Character Position Database Object
Renamed Position types to Positions to Match Ripley's original code. Looks better in character, also.
Changed to the correct function when Executing constructed statements
Added another debug command
Added Position update to the Character Data object
Changed the positiontype to a keyed column in mysql
DB Updates
Morning updates - working position inserts at character creation
Updated constructstatement, positions are saving to the right columns
Load all available character positions from the database or create a new if missing
Fixed unused GetCharacterPhysicalPosition
Made the server use the correct insert function
Fixed position save
Cleaned up code and removed the unnecessary SQL statement
Moved Character Prepared Statement for Selecting position to the ConstructedStatement Get function
Updated changelog

fantoms · 2017-03-30T02:51:28Z

Source/ACE/Entity/Player.cs

+        /// <summary>
+        /// Set's the tied lifestone for use casting the Lifestone Recall spell
+        /// </summary>
+        public void SetLifestoneTiedCharacterPosition(CharacterPosition newPosition)


This begins the stubs for functions we will write in the future.

It seems like we should have a single "SetPosition(Enum.Position positionType, Position position)" method so that we don't have to have 1 of these for each.

fantoms · 2017-03-30T02:52:02Z

Source/ACE/Entity/Player.cs

-                character.Position = Session.Player.Position;
-
+                // Save the current position to persistent storage
+                SetPhysicalCharacterPosition();


Loads the Player position into the dictionary of positions before saving in the database.

fantoms · 2017-03-30T02:53:07Z

Source/ACE/Network/Handlers/CharacterHandler.cs

@@ -132,6 +132,7 @@ public static void CharacterRestore(ClientPacketFragment fragment, Session sessi
            uint lowGuid = DatabaseManager.Character.GetMaxId();
            character.Id = lowGuid;
            character.AccountId = session.Id;
+            character.DateOfBirth = character.CreationTimeStamp.ToString();


This field was always null before this change.

check /birth command, I'm thinking this property was the same output

fantoms · 2017-03-30T02:54:28Z

Source/ACE/Network/Handlers/CharacterHandler.cs

@@ -140,7 +141,9 @@ public static void CharacterRestore(ClientPacketFragment fragment, Session sessi
            }

            CharacterCreateSetDefaultCharacterOptions(character);
+            CharacterCreateSetDefaultCharacterPositions(character);


Set's the initial values for the character positions, when the character is first created.

fantoms · 2017-03-30T02:55:41Z

Source/ACE/Command/Handlers/DebugCommands.cs

+        /// <param name="session"></param>
+        /// <param name="parameters"></param>
+        [CommandHandler("save", AccessLevel.Developer, CommandHandlerFlag.RequiresWorld)]
+        public static void Save(Session session, params string[] parameters)


Useful for invoking a database save; I used this instead of logging out or waiting 5 minutes.

There's already a save command in AdminCommands, suggest renaming this to something else

fantoms · 2017-03-30T02:56:51Z

Source/ACE/Command/Handlers/DebugCommands.cs

+        /// <param name="session"></param>
+        /// <param name="parameters"></param>
+        [CommandHandler("whoami", AccessLevel.Developer, CommandHandlerFlag.RequiresWorld)]
+        public static void WhoAmI(Session session, params string[] parameters)


With multiple characters on screen/per account, this is helpful when comparing columns in the database.

fantoms · 2017-03-30T02:57:29Z

Source/ACE.Entity/Enum/CharacterPositionType.cs

@@ -0,0 +1,139 @@
+namespace ACE.Entity.Enum
+{
+    public enum CharacterPositionType


Class to hold character specific positions.

As opposed to the other Positions enum? are they both needed?

Updated comments on CharacterPositionType Added the sql for altering the positions table, to add the positionType Added ripleys Position Types Added Character Position Database Object Renamed Position types to Positions to Match Ripley's original code. Looks better in character, also. Changed to the correct function when Executing constructed statements Added another debug command Added Position update to the Character Data object Changed the positiontype to a keyed column in mysql DB Updates Morning updates - working position inserts at character creation Updated constructstatement, positions are saving to the right columns Load all available character positions from the database or create a new if missing Fixed unused GetCharacterPhysicalPosition Made the server use the correct insert function Fixed position save Cleaned up code and removed the unnecessary SQL statement Moved Character Prepared Statement for Selecting position to the ConstructedStatement Get function Updated changelog

fantoms · 2017-03-30T03:12:55Z

Source/ACE.Database/CharacterDatabase.cs

            AddPreparedStatement(CharacterPreparedStatement.CharacterSkillsSelect, "SELECT `skillId`, `skillStatus`, `skillPoints`, `skillXpSpent` FROM `character_skills` WHERE `id` = ?;", MySqlDbType.UInt32);
            AddPreparedStatement(CharacterPreparedStatement.CharacterStatsSelect, "SELECT `strength`, `strengthXpSpent`, `strengthRanks`, `endurance`, `enduranceXpSpent`, `enduranceRanks`, `coordination`, `coordinationXpSpent`, `coordinationRanks`, `quickness`,  `quicknessXpSpent`, `quicknessRanks`, `focus`, `focusXpSpent`, `focusRanks`, `self`, `selfXpSpent`, `selfRanks`, `healthRanks`, `healthXpSpent`, `healthCurrent`, `staminaRanks`, `staminaXpSpent`, `staminaCurrent`, `manaRanks`, `manaXpSpent`, `manaCurrent` FROM `character_stats` WHERE `id` = ?;", MySqlDbType.UInt32);
            AddPreparedStatement(CharacterPreparedStatement.CharacterAppearanceSelect, "SELECT  `eyes`, `nose`, `mouth`, `eyeColor`, `hairColor`, `hairStyle`, `hairHue`, `skinHue` FROM `character_appearance` WHERE `id` = ?;", MySqlDbType.UInt32);
            AddPreparedStatement(CharacterPreparedStatement.CharacterFriendsSelect, "SELECT cf.`friendId`, c.`name` FROM `character_friends` cf JOIN `character` c ON (cf.`friendId` = c.`guid`) WHERE cf.`id` = ?;", MySqlDbType.UInt32);

-            AddPreparedStatement(CharacterPreparedStatement.CharacterPositionInsert, "REPLACE INTO character_position (id, cell, positionX, positionY, positionZ, rotationX, rotationY, rotationZ, rotationW) VALUES (?, ?, ?, ?, ?, ?, ? ,? ,?);", MySqlDbType.UInt32, MySqlDbType.UInt32, MySqlDbType.Float, MySqlDbType.Float, MySqlDbType.Float, MySqlDbType.Float, MySqlDbType.Float, MySqlDbType.Float, MySqlDbType.Float);
+            ConstructStatement(CharacterPreparedStatement.CharacterPositionSelect, typeof(CharacterPosition), ConstructedStatementType.Get);


Converted Get and added the Insert/Update constructedstatements.

fantoms · 2017-03-30T03:19:33Z

Source/ACE.Database/CharacterDatabase.cs

@@ -173,7 +223,11 @@ public bool IsNameAvailable(string name)

        public async Task UpdateCharacter(Character character)
        {
-            ExecutePreparedStatement(CharacterPreparedStatement.CharacterPositionInsert, character.Id, character.Position.LandblockId.Raw, character.Position.Offset.X, character.Position.Offset.Y, character.Position.Offset.Z, character.Position.Facing.X, character.Position.Facing.Y, character.Position.Facing.Z, character.Position.Facing.W);
+            // Save all of the player positions
+            foreach(var pos in character.CharacterPositions)


This is a big hammer to save all during every save event.

Mogwai-TheFurry · 2017-03-30T03:14:28Z

Source/ACE.Database/CharacterDatabase.cs

+            CharacterPosition newCharacterPosition = new CharacterPosition();
+            if (result.Count > 0)
+            {
+                newCharacterPosition.cell = result.Read<uint>(0, "cell");


didn't want to use the ORM stuff here?

That was updated from a previous slide; I've pushed a commit with updated ORM calls.

Mogwai-TheFurry · 2017-03-30T03:15:09Z

Source/ACE.Entity/Character.cs

+            // initialize the blank character positions
+            InitializeCharacterPositions();
+            CharacterPositions = new ReadOnlyDictionary<CharacterPositionType, CharacterPosition>(characterPositions);
+            PhysicalPosition = new Position(CharacterPositions[CharacterPositionType.PhysicalLocation].cell, CharacterPositions[CharacterPositionType.PhysicalLocation].positionX, CharacterPositions[CharacterPositionType.PhysicalLocation].positionY, CharacterPositions[CharacterPositionType.PhysicalLocation].positionZ, CharacterPositions[CharacterPositionType.PhysicalLocation].rotationX, CharacterPositions[CharacterPositionType.PhysicalLocation].rotationY, CharacterPositions[CharacterPositionType.PhysicalLocation].rotationZ, CharacterPositions[CharacterPositionType.PhysicalLocation].rotationW);


seems like it would be helpful here to have a Position.Clone() method so that you didn't have to do this inline.

Mogwai-TheFurry · 2017-03-30T03:16:02Z

Source/ACE.Entity/CharacterPosition.cs

+
+namespace ACE.Entity
+{
+    [DbTable("character_position")]


was using the existing one just not going to work?

Mogwai-TheFurry · 2017-03-30T03:17:17Z

Source/ACE.Entity/Enum/CharacterPositionType.cs

@@ -0,0 +1,139 @@
+namespace ACE.Entity.Enum
+{
+    public enum CharacterPositionType


As opposed to the other Positions enum? are they both needed?

Mogwai-TheFurry · 2017-03-30T03:19:09Z

Source/ACE/Entity/Player.cs

+        /// <summary>
+        /// Set's the tied lifestone for use casting the Lifestone Recall spell
+        /// </summary>
+        public void SetLifestoneTiedCharacterPosition(CharacterPosition newPosition)


It seems like we should have a single "SetPosition(Enum.Position positionType, Position position)" method so that we don't have to have 1 of these for each.

Mogwai-TheFurry · 2017-03-30T03:21:09Z

Source/ACE.Database/CharacterDatabase.cs

+        /// <summary>
+        /// This function will save all of the CharacterPositions 
+        /// </summary>
+        public void SaveCharacterPositions(Character character)


I'm pretty sure we can just do 1-off saves when the player changes them. The only save that should be saved with the character is the physical position and maybe the last used portal. We can save the others when they tie/bind.

Mogwai-TheFurry · 2017-03-30T03:21:51Z

Source/ACE/Network/Handlers/CharacterHandler.cs

+            newStartingPosition.character_id = character.Id;
+            newInvalidPosition.character_id = character.Id;
+            character.SetCharacterPositions(CharacterPositionType.PhysicalLocation, CharacterPositionExtensions.StartingPosition);
+            character.SetCharacterPositions(CharacterPositionType.LifestoneUsed, CharacterPositionExtensions.InvalidPosition);


I don't think these are really necessary. I'm OK with having "null" for Positions the character has never tied to.

LtRipley36706 · 2017-03-30T03:45:55Z

Database/Updates/Character/2017_03_27_character_positions_change.sql

+ALTER TABLE `character_position`
+CHANGE COLUMN `id` `character_id` INT(10) UNSIGNED NOT NULL DEFAULT '0' FIRST;
+ALTER TABLE `character_position`
+	CHANGE COLUMN `positionType` `positionType` TINYINT(3) UNSIGNED NOT NULL DEFAULT '0' AFTER `cell`,


change AFTER to BEFORE

LtRipley36706 · 2017-03-30T03:46:11Z

Database/Updates/Character/2017_03_27_character_positions_change.sql

+	ALTER `positionType` DROP DEFAULT;
+ALTER TABLE `character_position`
+	CHANGE COLUMN `character_id` `character_id` INT(10) UNSIGNED NOT NULL FIRST,
+	CHANGE COLUMN `positionType` `positionType` TINYINT(3) UNSIGNED NOT NULL AFTER `cell`;


change AFTER to BEFORE

LtRipley36706 · 2017-03-30T03:46:51Z

Source/ACE.Database/CharacterDatabase.cs

@@ -260,12 +314,12 @@ public bool IsNameAvailable(string name)
                c.TemplateOption = result.Read<uint>(0, "templateOption");
                c.StartArea = result.Read<uint>(0, "startArea");

-                c.Position = await this.GetPosition(guid);
+                c.PhysicalPosition = await this.GetPosition(guid);


Change PhysicalPosition to Location?

Character.Location = current position right?

LtRipley36706 · 2017-03-30T03:47:45Z

Source/ACE.Database/Database.cs

-
+#if DBDEBUG
+                        Console.WriteLine(types);
+                        Console.WriteLine(query);


Change to log.Debug ?

LtRipley36706 · 2017-03-30T03:48:27Z

Source/ACE.Entity/Enum/CharacterPositionType.cs

+        /// <summary>
+        /// Allegiance hometown recall position
+        /// </summary>
+        AllegianceHometown = 0x06,


LtRipley36706 · 2017-03-30T03:48:33Z

Source/ACE.Entity/Enum/CharacterPositionType.cs

+        /// <summary>
+        /// Mansion recall position
+        /// </summary>
+        MansionRecall = 0x07


LtRipley36706 · 2017-03-30T03:53:36Z

Source/ACE/Entity/Player.cs

+            // TODO : Save the Mansion Recall position information from the destination of a house box
+            character.SetCharacterPositions(CharacterPositionType.MansionRecall, newPosition);
+        }
+


Not sure we'd set Allegiance Hometown and Mansion Recall positions to each character.. Mansion recall is essentially a copy of monarch's house recall if house is a villa or mansion, otherwise it didn't work or errored, not sure what it did here. I assume it would be a check if character is in allegiance, followed by confirming house type, then execution or returning error.

Hometown recall might be stored in the allegiance information somewhere, i haven't seen it yet however.

LtRipley36706 · 2017-03-30T03:54:27Z

Source/ACE/Network/Handlers/CharacterHandler.cs

@@ -132,6 +132,7 @@ public static void CharacterRestore(ClientPacketFragment fragment, Session sessi
            uint lowGuid = DatabaseManager.Character.GetMaxId();
            character.Id = lowGuid;
            character.AccountId = session.Id;
+            character.DateOfBirth = character.CreationTimeStamp.ToString();


check /birth command, I'm thinking this property was the same output

LtRipley36706 · 2017-03-30T04:05:35Z

Database/Updates/Character/2017_03_26_character_positions_positiontype.sql

@@ -0,0 +1,2 @@
+ALTER TABLE `character_position`
+	ADD COLUMN `positionType` TINYINT UNSIGNED NULL DEFAULT '0' AFTER `cell`;


change AFTER to BEFORE

…code

…er loading

fantoms · 2017-03-30T15:56:12Z

I am going to close this PR, untill I work the Positions and CharacterPositions in too the same object and the code ready for another look.

Thanks For your reviews so far!

fantoms commented Mar 30, 2017

View reviewed changes

fantoms force-pushed the feature-CharacterPositions branch from 0653862 to 0fda5e0 Compare March 30, 2017 03:10

fantoms commented Mar 30, 2017

View reviewed changes

Mogwai-TheFurry suggested changes Mar 30, 2017

View reviewed changes

LtRipley36706 requested changes Mar 30, 2017

View reviewed changes

LtRipley36706 reviewed Mar 30, 2017

View reviewed changes

fantoms added 2 commits March 29, 2017 23:24

Changed the LoadCharacterPositions to ORM OUT of the database.

7a0bf00

Changed the introduced @save command to @Save-Now to avoid conflict

63df763

fantoms force-pushed the feature-CharacterPositions branch from be3bb01 to 63df763 Compare March 30, 2017 04:58

fantoms added 4 commits March 30, 2017 00:51

Changed CharacterPositionType to PositionTypes and moved in Ripley's …

3b0703c

…code

Changed PhyiscalPosition to Location

f046ca1

Removed plural in function name for position types; broke the charact…

3eb31a5

…er loading

Added GetList statement for all positions

5a58c7c

fantoms closed this Mar 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: ORM Driven Character Positions for Issue #166 #184

Feature: ORM Driven Character Positions for Issue #166 #184

fantoms commented Mar 30, 2017

fantoms Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

fantoms Mar 30, 2017

fantoms Mar 30, 2017

LtRipley36706 Mar 30, 2017

fantoms Mar 30, 2017

fantoms Mar 30, 2017

LtRipley36706 Mar 30, 2017

fantoms Mar 30, 2017

fantoms Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

fantoms Mar 30, 2017

fantoms Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

fantoms Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

Mogwai-TheFurry Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

LtRipley36706 Mar 30, 2017

fantoms commented Mar 30, 2017

		@@ -0,0 +1,2 @@
		ALTER TABLE `character_position`
		ADD COLUMN `positionType` TINYINT UNSIGNED NULL DEFAULT '0' AFTER `cell`;

Feature: ORM Driven Character Positions for Issue #166 #184

Feature: ORM Driven Character Positions for Issue #166 #184

Conversation

fantoms commented Mar 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fantoms commented Mar 30, 2017