[WIP] Core/Misc Support for Account Wide Mounts #14849

Boomper · 2015-06-07T01:51:04Z

Everything from TODO list seems to be finished (except playercondition.db2, no idea how that works, I'll leave it to someone else) so feel free to test it out and leave feedback.

Edit: Currently I'm stuck with 6.2 client unable to downgrade it/use previous versions (#134 error) so waiting till the core get's updated.

- it's still work in progress, I'm just tired from this weird weather so I'd like some feedback before I mess something up... - the packet size is based on https://gist.github.com/Boomper/4434a33ac3c0f10dd1ee packet length was 476 and if you fill it with numbers (1 + 4 + 4 + 452 + 14 + 1) it equals 476 aswell... so it propably makes sense... - TODO: - Evil SQL Query to transfer old mounts from character_spell into account_mounts (I'll propably have to export the Mount.db2 for spellIds (just thinking out loud) - add support for PlayerCondition.db2 - add support for db table disables - add support for mounts that change when you change your faction (propably table in world db player_factionchange_spells)

Vincent-Michael · 2015-06-07T01:57:18Z

src/server/game/Server/Packets/MiscPackets.h

+            void InitializeMounts(std::unordered_map<uint32, bool> mounts, bool fullUpdate);
+            WorldPacket const* Write() override;
+
+            bool IsFullUpdate;


bool IsFullUpdate = false;

ElleEntertainment · 2015-06-07T08:32:50Z

gj

Shauren · 2015-06-07T08:48:08Z

You are executing database queries directly in packet handlers (MountSetFavorite) - this is unacceptable as it opens a DoS attack possibility. I suggest you take a look at how everything else is saved for players and add it there (Player::SaveToDB)

Another thing is that mounts are battle.net account wide what adds another layer of complexity since you can be logged on the account multiple times (many game accounts) - data needs to be synchronized between them (possibly across multiple realms)

- actual "save system" - account_mounts table is now moved into LoginDatabase (auth)

- forgot to move the sql file...

jackpoz · 2015-06-08T15:02:00Z

src/server/game/Entities/Player/Player.cpp

@@ -3420,6 +3420,9 @@ bool Player::AddSpell(uint32 spellId, bool active, bool learning, bool dependent
        return false;
    }

+    if (MountEntry const* mountEntry = sDB2Manager.GetMount(spellId))


mountEntry variable looks unused

Boomper · 2015-06-08T15:29:12Z

MountEntry is there for "Todo: add support for PlayerCondition.db2" because it contains value "PlayerConditionId" which is for mounts that you can use only after you complete quest/achievement/something...

There is no check if spells referenced in Mount.db2 exist, or I'm being blind.

Currently the loading part isn't working, must've messed something up when I've moved the table into LoginDatabase...

Boomper · 2015-06-09T20:56:37Z

Just finished writing the "Evil SQL Query to transfer old mounts from character_spell into account_mounts"...
https://gist.github.com/Boomper/ea3f851f66ddf0f8f1eb
Since it's "touching" auth, characters and world database (multidb queries) should it still be in sql/updates/auth or is there a special place for it?

I'll be updating the pull request in few hours, just need to polish some stuff (the loading should be fixed by then)...

Aokromes · 2015-06-09T21:27:18Z

The query must be on the final destination database.

- now also merges all spells from character_spell into account_mounts - not going to commit other things tonight, my computer is being mean by refusing to compile core under 30 minutes so rest will be tomorrow (today 10.6.2015) night-ish

Boomper · 2015-06-09T23:28:42Z

sql/updates/auth/2015_MM_DD_NN_auth.sql

+JOIN `characters`.`characters` `chars` ON `chars`.`guid` = `characters`.`character_spell`.`guid`
+WHERE `characters`.`character_spell`.`spell` IN (SELECT `spell` FROM `temp_mounts`);
+
+DELETE FROM `characters`.`character_spell` WHERE `spell` IN (SELECT `spell` FROM `temp_mounts`);


@Aokromes I was writing about this. The reason why I don't want it in a seperate sql file is that it deletes the now useless mounts from character_spell and uses temp_mounts table which is dropped afterwards... -> it wouldn't do anything because the table temp_mounts wouldn't exist.
Yes I could just do "WHERE spell IN (1, 2,...)" but that would open up the possiblity that someone somewhere could execute the "DELETE FROM..." query before this one which would make him loose all playermounts.

Maybe the way to do is a secondary commit after few days on that way no one fucks the database xd

- loading from auth DB now works - but I need to look into the asynch loading for smoother experience (can't quite figure out how that works for login db, any pointers would be appreciated (I don't mean pointers like 0x2E314A81)) - added basic handling for faction mounts (through world db table playerfactionchange_spells), players now learn both version (alliance and horde) but only the version for their faction will appear in their mount collection (there are exceptions because some faction based mounts aren't included in that table - also added m_mountCount variable into Player class for future handling of mount count achievements (+ for size of vectors with sweet packet data (this text doesn't have to make sense, I'm tired))

- whooooops... hopefuly no one notices...

Boomper · 2015-06-10T00:47:20Z

Wrongly merged something, I'll fix it during sunlight...

ghost · 2015-06-10T02:05:05Z

src/server/game/Entities/Player/Player.cpp

+    if (!sSpellMgr->GetSpellInfo(spellId))
+        return false;
+
+    if (!sDB2Manager.GetMount(spellId))


check again?

jackpoz · 2015-06-10T08:56:58Z

Just finished writing the "Evil SQL Query to transfer old mounts from character_spell into account_mounts"...
https://gist.github.com/Boomper/ea3f851f66ddf0f8f1eb

lines like

DROP TABLE IF EXISTS `auth`.`account_mounts`;

don't assure in ANY way that the statement is executed on the correct database and should NEVER be written in that way (speaking of personal experience, in my case "auth" is 335 auth database and "auth6x" is 6.x auth database, so your sql will execute statements on the 335 database instead of the 6.x one)

This is just the issue and it doesn't include the solution. 919ab91 is what I did last time I encountered this issue, up to you (and other devs) about what to do.

Take a look how the solution plays with TC Database Auto Update too.

Warpten · 2015-06-10T09:03:42Z

sql/updates/auth/2015_MM_DD_NN_auth.sql

+JOIN `characters`.`characters` `chars` ON `chars`.`guid` = `characters`.`character_spell`.`guid`
+WHERE `characters`.`character_spell`.`spell` IN (SELECT `spell` FROM `temp_mounts`);
+
+DELETE FROM `characters`.`character_spell` WHERE `spell` IN (SELECT `spell` FROM `temp_mounts`);


the problem with this sql file is that you directly reference world, chars, characters (dafuq?) databases in this file, while there is no guarantee for anyone that all databases use those names

Or even that these databases are on the same server

From now on I shall reference them through time-space rift..., removing this from the pull request in next update, thanks for those heartwarming lines with so much potential to help me with this.

- wrongly merged files from yesterday (did some cleanup in my local branches, from now on there shouldn't be issues like this)

- it's going to be replaced with space-time rift which will convert all the old mounts in character_spell into those new ones (long story short: I'm not going to do anything with this until someone suggests better way)

Boomper · 2015-06-10T16:52:42Z

Fixed the compilation errors from yesterday (problems with my local branches), removed the conversion part from SQL file and updated the description.

- added new sqlqueryholder for other account wide things like achievements, battle pets, quets,... - accepting suggestions for better name for "collectionsHolder"

… (account_mounts) - old mounts are now added into new ones when player enters the world - no idea how that async query converted itself into sync query... it should be async... weird

Boomper · 2015-06-12T16:34:41Z

The merging of old mounts is now handled by core (when player enters the world), I think that's all I was planning on doing so can someone test this if they find something wrong with it so we can finish this?

ghost · 2015-06-13T09:53:07Z

Nice work, keep it :)

Boomper · 2015-06-15T19:20:59Z

So I did some more research into the account wide mounts system and it seems like that @Exodius was right. I'm currenlty trying to write a list for sql table for mounts that should be account-wide/account-wide in spellbook/and only character specific...

Shauren · 2015-06-15T21:43:15Z

@Boomper don't you dare do that. The client is able to know this by itself (see ingame mount tooltips) - fd786c8

Also a list of mount flags I conveniently researched earlier https://github.com/TrinityCore/TrinityCore/blob/6.x/src/server/game/DataStores/DBCEnums.h#L578

Boomper · 2015-06-16T05:45:20Z

@Shauren Completely missed that one, thanks 👍
I did found that there are few mount "pairs" like:
http://www.wowhead.com/item=44226 / http://www.wowhead.com/item=44225 (not sure why) http://www.wowhead.com/item=67101 / http://www.wowhead.com/item=62298 (unlocked by completing achievement)
That work in a way that If you learn one of them you also learn the second one and you're only able to see one of them in your mounts (depending on your faction).
So what I'm currently thinking is that I should still make a table to list these mounts, and maybe in same table I should keep track of faction for which the mount works (doesn't seem to be stored in DBC) and temporarily raceMask/classMask till playercondition.db2 is implemented.

Shauren · 2015-06-18T22:44:35Z

Just a heads up - I am working on implementing something nice for loading account wide data so you will need to adjust your patch in a near future

Boomper · 2015-06-22T02:02:44Z

https://gist.github.com/Boomper/e6a2c9abc45512ab5364
That's the table that I'm proposing (accepting suggestions for better table/variable names), how it should work:

spell (spell id of the mount)
faction (0 - factionism free; 1 - alliance only; 2 - horde only) - if the mount is horde only on alliance character, he/she will see it in his collections but it will be red (unusable)
other_spell (oposite version) - those are special cases when the player gets both from earning one of them but can only see one of them (his faction version) in his collection
raceMask (1 Human | 2 Orc |...) - these mounts will only appear in collection if player fits into raceMask
classMask (same as raceMask)
I should have the table implemented mid-week if I get enough free time.

P.S.: Too tired, going to cry... so much gold/time wasted on testing.

Shauren · 2015-06-22T07:30:08Z

other_spell (oposite version) - those are special cases when the player gets both from earning one of them but can only see one of them (his faction version) in his collection

Can you give an example of this?

Also please don't add class/race/faction conditions if they can be pulled from PlayerCondition.db2 (even if it's not implemented)

Boomper · 2015-06-22T09:51:28Z

http://www.wowhead.com/spell=60424 Mekgineer's Chopper / http://www.wowhead.com/spell=55531 Mechano-Hog
It's mostly mounts that are rewarded from completing faction specific reputations/achievements/quests or mounts that are faction specific and award achievement for getting them (mekgineer's chopper/mechano-hog - achievement http://www.wowhead.com/achievement=2097)

Edit: Currently I'm stuck with 6.2 client unable to downgrade it/use previous versions (#134 error) so waiting till the core get's updated.

- just adding this so I can update my repo for 6.2 so I can finally test this out

- TODO: Research how to squish commits... for reasons... it looks cooler

Aokromes · 2015-09-13T08:36:42Z

All checks have failed
1 failing check
Details continuous-integration/travis-ci/pr — The Travis CI build failed
This branch has conflicts that must be resolved
Use the command line to resolve conflicts before continuing.

Boomper · 2015-09-13T10:33:40Z

I've been busy with exams and studying for them, no idea when I'll have time to finish this.

ghost · 2015-09-13T11:49:32Z

To fix the conflicts, you will usually only need to update/rebase your branch so it is even with 6.x
(just make sure that you got a backup copy of your files, in case anything goes wrong during rebase).

MitchesD · 2015-11-13T12:44:48Z

I am closing this PR:

It's outdated
I am working on it ;)
Author looks like gone

Boomper · 2015-11-14T11:35:08Z

Go for it, feel free to reuse whatever you want from it, if there's something worth a while.
I don't have time to work on this anymore.

Vincent-Michael reviewed Jun 7, 2015
View reviewed changes

Aokromes added Comp-Core Branch-master labels Jun 7, 2015

Boomper added 2 commits June 7, 2015 17:53

Core/Misc Account Wide Mounts

a44563d

- actual "save system" - account_mounts table is now moved into LoginDatabase (auth)

DB/Misc TYPO

200623a

- forgot to move the sql file...

jackpoz reviewed Jun 8, 2015
View reviewed changes

jackpoz added the Feedback-FixOutdatedMissingWIP label Jun 9, 2015

Boomper reviewed Jun 9, 2015
View reviewed changes

Boomper added 2 commits June 10, 2015 02:03

TYPO

fc1450e

- whooooops... hopefuly no one notices...

ghost reviewed Jun 10, 2015
View reviewed changes

Warpten reviewed Jun 10, 2015
View reviewed changes

Boomper added 2 commits June 10, 2015 18:39

Core/Misc TYPO

5d0c400

- wrongly merged files from yesterday (did some cleanup in my local branches, from now on there shouldn't be issues like this)

DB/Misc Remove of conversion part of query for account_mounts table

3123125

- it's going to be replaced with space-time rift which will convert all the old mounts in character_spell into those new ones (long story short: I'm not going to do anything with this until someone suggests better way)

Boomper added 2 commits June 12, 2015 00:08

Core/Player Async Loading of Account-Wide Mounts

bc69c2e

- added new sqlqueryholder for other account wide things like achievements, battle pets, quets,... - accepting suggestions for better name for "collectionsHolder"

Core/Player Convesion of old (character_spell) mounts into new mounts…

ce34a29

… (account_mounts) - old mounts are now added into new ones when player enters the world - no idea how that async query converted itself into sync query... it should be async... weird

Boomper added 2 commits July 18, 2015 02:12

Core/Something Support for mount definitions

606f5b5

- just adding this so I can update my repo for 6.2 so I can finally test this out

Merge to f45ae7a

38c5a14

- TODO: Research how to squish commits... for reasons... it looks cooler

MitchesD closed this Nov 13, 2015

ghost mentioned this pull request Jan 10, 2016

Core/Scripts: Core support for Heroic Leap #14807

Closed

Aokromes removed the Feedback-FixOutdatedMissingWIP label Aug 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Core/Misc Support for Account Wide Mounts #14849

[WIP] Core/Misc Support for Account Wide Mounts #14849

Boomper commented Jun 7, 2015

Vincent-Michael Jun 7, 2015

ElleEntertainment commented Jun 7, 2015

Shauren commented Jun 7, 2015

jackpoz Jun 8, 2015

Boomper commented Jun 8, 2015

Boomper commented Jun 9, 2015

Aokromes commented Jun 9, 2015

Boomper Jun 9, 2015

Aokromes Jun 10, 2015

Boomper commented Jun 10, 2015

ghost Jun 10, 2015

jackpoz commented Jun 10, 2015

Warpten Jun 10, 2015

Shauren Jun 10, 2015

Boomper Jun 10, 2015

Boomper commented Jun 10, 2015

Boomper commented Jun 12, 2015

ghost commented Jun 13, 2015

Boomper commented Jun 15, 2015

Shauren commented Jun 15, 2015

Boomper commented Jun 16, 2015

Shauren commented Jun 18, 2015

Boomper commented Jun 22, 2015

Shauren commented Jun 22, 2015

Boomper commented Jun 22, 2015

Aokromes commented Sep 13, 2015

Boomper commented Sep 13, 2015

ghost commented Sep 13, 2015

MitchesD commented Nov 13, 2015

Boomper commented Nov 14, 2015

[WIP] Core/Misc Support for Account Wide Mounts #14849

[WIP] Core/Misc Support for Account Wide Mounts #14849

Conversation

Boomper commented Jun 7, 2015

Choose a reason for hiding this comment

ElleEntertainment commented Jun 7, 2015

Shauren commented Jun 7, 2015

Choose a reason for hiding this comment

Boomper commented Jun 8, 2015

Boomper commented Jun 9, 2015

Aokromes commented Jun 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Boomper commented Jun 10, 2015

Choose a reason for hiding this comment

jackpoz commented Jun 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Boomper commented Jun 10, 2015

Boomper commented Jun 12, 2015

ghost commented Jun 13, 2015

Boomper commented Jun 15, 2015

Shauren commented Jun 15, 2015

Boomper commented Jun 16, 2015

Shauren commented Jun 18, 2015

Boomper commented Jun 22, 2015

Shauren commented Jun 22, 2015

Boomper commented Jun 22, 2015

Aokromes commented Sep 13, 2015

Boomper commented Sep 13, 2015

ghost commented Sep 13, 2015

MitchesD commented Nov 13, 2015

Boomper commented Nov 14, 2015