[8874][patch] Extract player skills from data blob

[begemot]

Which disadvantages do you see in current position of patch object ?

And why current system was done in such way ?

[XTZGZoReX]

And why current system was done in such way ?

Because it was just made to "work". Nothing else.

[darkstalker]

why not make a table "player_data" with fields: (playerguid, field, value)

would be a lot easier to mantain and update

[voidx]

+1 xDDDD and items_instace same.... this tipe of tables is a big problem on eficience of db...

[The_Game_Master]

Bump!

Please update!

[begemot]

Because it was just made to "work". Nothing else.

And I thought that it was so because close to Bliz. data structures...

And because in case of split field "data" into several fields - there will be too many fields... may be it will bring DB to slower work and some other problems...

If you have to do some DB just make to "work" will you do it such hard way ?

[XTZGZoReX]

Ofc we will still just enumerate some data values into blob fields even after we do cleanup. Otherwise, we'd get way too many fields.

[LordJZ]

With all values saved in data blob there can be several issues with saving and loading them.

I.e. aura #73 is applied, values in data blob are changed (increased/decreased) but not changed back when the aura fades during character offline time. Sounds pretty strange, but it is.

[seirge]

why not make a table "player_data" with fields: (playerguid, field, value)

would be a lot easier to mantain and update

And It would be much slower to load into core.

However, does _this_ patch has any problems now? Is it requires testing or there are some issues why it is not accepted?

[VladimirMangos]

As i remember only thing that stop apply it early has been unfinished discussion about effectiveness by memory use suggested structures. Note: in fact patch also expected speedup work code because lookup skill data is very often operation.

[Darky88]

Some fix for the save crash?

Att Darky

[xeross155]

One could use this for multi spec and then simply save the spec ID with the skill.

[silviu2009]

Sorry, an error has occurred. Reason: That is an invalid ID, or the post has expired.

Can you re-upload the patch please? Thanks!

EDIT: My bad, didn't see all posts!

[seirge]

The patch will lead to server hang up when trying to save SKILL_UNCHANGED skill. The following patch will fix it.

diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index fadfc43..38493d5 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -16273,8 +16273,11 @@ void Player::_SaveSkills()
    // we don't need transactions here.
    for( SkillStatusMap::iterator itr = mSkillStatus.begin(); itr != mSkillStatus.end(); )
    {
-        if(itr->second.uState == SKILL_UNCHANGED)
+        if(itr->second.uState == SKILL_UNCHANGED)
+        {
+            ++itr;
            continue;
+        }

        if(itr->second.uState == SKILL_DELETED)
        {

[hunuza]

Thank you and especially seirge.

Applied in [8874].

[The_Game_Master]

Just one question. I've seen that the skills are inserted into character_skills, that means that they are deleted from the data blob right?

I'm just asking becouse i haven't seen anything like this in here.

http://github.com/mangos/mangos/blob/5151306ea49eb3bc1d4feee4d574edc30c096cb0/sql/updates/8874_01_characters_character_skills.sql

[hunuza]

For now they are still saved in the data blob, but in the future we want to remove the data blob from the database.