cmangos / issues

This repository is used as a centralized point for all issues regarding CMaNGOS.
179 stars 47 forks source link

🚀 [Git Repo Combination Discussion] - Efficiency/Modernization of Working Practices (6 Repos -> 3 Repos) #2374

Open MantisLord opened 3 years ago

MantisLord commented 3 years ago

Idea: What do you guys think about combining each content database git repo with their respective core git repos? https://github.com/cmangos/classic-db -> https://github.com/cmangos/mangos-classic https://github.com/cmangos/tbc-db -> https://github.com/cmangos/mangos-tbc https://github.com/cmangos/wotlk-db -> https://github.com/cmangos/mangos-wotlk

With this approach, there would be no confusion about whether a SQL change belongs in the "core" or "database" repository. All issues sections and pull requests here on github such as https://github.com/cmangos/tbc-db/issues would be migrated to a central place per expansion (the three core repositories).

When's the last time we considered doing something like this to cut down on headaches for new contributors and maintainers? Ideally you shouldn't need clone/juggle two separate repos always in order to get going with the software.

We should also change our updates/migrations system to more closely match that of Nostalrius/VMaNGOS. This way we don't have to rely on the column renaming approach we do currently.

Every SQL update is a migration in Nost core and it all goes into the same place: https://github.com/vmangos/core/tree/development/sql/migrations They have a helper script to create new migrations files like this: https://github.com/vmangos/core/blob/development/sql/make_migration.bat Then there is a simple script included to combine the latest migrations of the current DB release, just how we have our db install scripts currently, but more simplistic: https://github.com/vmangos/core/blob/development/sql/migrations/merge.bat

The result is a cleaner change history where you can see both DB and C++ changes that relate to each other being made in the same place/commit as you would expect for basically any project like ours. See example of what I mean here: https://github.com/vmangos/core/commit/9567ba0d7944a883243c653364b40195f50ece83

This is what Trinity based projects and original mangos seem to have done a very long time ago - that is, combine the "database projects" into the "core project". I don't think there's really a risk to this from a legal standpoint, considering how long all these other projects have been operating like this. There's such laughably large chunk of things missing and broken if you decide you still want to run cmangos separately from a content database like classic-db/tbc-db/wotlk-db in 2021.... Surely there was a time back when ACID/EAI and ScriptDev2 were all designed to be modular/separated into their own organizations when this made much more sense to do... I think the time has long past for this, along with various other parallel world database efforts like "YTDB" or "Silverpine".

canidae commented 3 years ago

I'm primarily just a user with limited contribution to the project, but I'll chime in with my thoughts if they're of any value:

I support the notion of joining database with core. This always seemed like an unnecessary complication that made it more difficult both to install the server with a database as well as contributing something that requires changes simultaneously in core and database. Updating the database seems like something that could be improved. More automation and less user interaction would probably lead to fewer issues with incorrect databases.

I know this discussion is about joining database and core, but I'd still like to entertain the thought of going even further: I've wondered if it would be an improvement to combine the three cores into a single project, using build flags and preprocessor to distinguish which core you're building. There seems to be quite a lot of code that's identical between the cores. Being primarily a user who can contribute with minor changes, I'd rather work with core/db for a single version than checking out 6 projects to verify that my changes are working correctly in all three cores. I believe that if the code contains e.g. "#ifdef WOTLK_CORE ... #else ... #endif", then it would be pretty obvious that the cores differ in functionality. If you're working on only Classic core, this would not be obvious. Even testing your changes on all cores would be less hassle, you would only need to rebuild the project (and use another client and databases), you don't need to clone more projects and patch in your changes.

killerwife commented 3 years ago

A no from me on that front. Maintaining an external core with this is going to be a nightmare. I am already sifting through thousands of commits. However I do admit its not really a problem for cmangos people, but it might make me not want to export commits anymore. (those concerned should try staring at a commandline for 4 hours at a time and they will understand what I mean)

cyberium commented 3 years ago

Mixing core and content db

You know iam always for everything that will help end users to build the server. So i considered that even myself more than one time. We are still in development and i think its not a good idea to mix content stuff with core stuff. The benefit for having db in same repo than core is not so big when you think about it.

Counter proposal

However, i would propose the opposite. Migrating all sql from core to db. I know you dont care too about having character db and realmdb in your repos but it will address some of the problem you pointed out isn't it? (Although realmdb should have in some future its own repo with the realmd core and be unified for all core version)

Migration

I dont think its more clear to have those migration files. Maybe i missed the point of them but its not fundamentally different of what we do in less clear way? Having c++ and sql mixed is not what i can call clear stuff. Ok the change are related but having 2 commits one for the core one for db is more clear to me. I think the most important for content db is to release fulldb often enough to not have 15000 patch files to apply. But even in that case install script handle them without any issue. Feel free to come back to me if you think i didn't get their purpose well enough.

Unify all core on same repo

Already proposed many time, take file like spell.cpp (but many other files are in same case) and try to identify all differences you'll see there its way too much to handle that with preprocessor. And even if it was possible its not A solution would be to maybe have entire game folder for each core but that will add many redundancies anyway. Its not a no but it will require lot of work that is not focused on server improvement itself to have it done in a way that will not result in worst situation that we have for user and devs.

MantisLord commented 3 years ago

I agree with the sentiments of @killerwife and @cyberium that attempting to combine all three cores into a single project now given the current state of development is not such a great idea. I don't think we should go that far in this discussion and instead limit the scope some. That's why I mainly want to talk about how we manage our three separate content database repos & data in conjunction with their three respective core repositories. Considering the amount of shared code between all three cores/databases/expansions, this is certainly still a valid discussion to have though at a later date/stage in development.

@cyberium's counter proposal is a very good one. Maybe we could investigate more into this idea sometime in the future. The main point of confusion for some contributors I've seen is having to figure out the answer to the question, "Where do my SQL changes/fixes belong?" Why do I need to fill the CORE_PATH config variable in my DB installation script in order to complete full setup of my world DB? We currently have all kinds of data like original DBC data stored in the core repository (Spell.sql file is a big example). In fact, this is where Broadcast texts are stored; whether they should be or not is a different question... These are all the actual texts for almost all the content in the game - so my point is, we are already inherently mixing DB content into core repo everywhere, whether we like it or not: https://github.com/cmangos/mangos-tbc/tree/master/sql

Not only should 0 of this DBC related/brute-forced data be stored in the core repo, I think ultimately the Cores should have nothing to do with even loading DBC data directly from any DBC files. It's 2021 and we have a full picture of all the DBC data from every file of every client build in existence... https://wow.tools/dbc/ Every DBC should work like spell_template does - we should extract all DBC content we require to run CMaNGOS into database tables that are loaded at the time of startup. Blizzard never designed DBC files to be read in directly by the server software. These files are formatted in such a way that they are tools for the game client to use, not the server. There's little doubt in my mind that Blizzard had a conventional database on their end to handle all of this type of thing. Due to the unchanging/static nature of this data, we should create a single "DBC database" for each version of CMaNGOS that would encompass a simple subset of all the extracted SQL from DBC files we need to reference for the final client build of each game version - 1.12, 2.4.3, and 3.3.5.

Not only would this eliminate alot of unnecessarily complex core/C++ code used to read the DBC specific formats, it would enable further customizations and flexibility like having spell_template editable in SQL has done for us... No one wants to edit a DBC file... Crafting a normal SQL update/insert statement is just about 1000x easier and more intuitive in comparison.

cyberium commented 3 years ago

I took some time to list all used dbc per core

i think the size is reasonable enough with 17Mo to add in server starting request for wotlk. So adding all of those data to db should be possible but will for sure slow down the server start because its faster to load those data from the file. The argument about removing some code for dbc reading is not valid. That code is here from long time enough and almost never been changed. The dbc format is well know and easy to read. Adding a new database for that seem to be an overkill, i would prefer to just have them in the world db prefixed by dbc_

Having all sql in db repository was not possible before, mostly because we wanted to keep that as a separate project developed by independant team. Iam not sure how many db are still actively developed nowadays. "We" as core devs also should keep db scheme control as in the end we have to handle them in core in predictable way. So dbdev was not allowed to change db scheme in any way if they wanted to keep db working. Also revision was done to be sure db was updated for core scheme changes. Not for any changes in contents. That should be keept as it is.

So 2 points:

Time to ask if all are agree to that and who will work on it.

Wotlk: 106 files 16.9Mo
AreaTable.dbc
Achievement.dbc
Achievement_Criteria.dbc
AreaTrigger.dbc
AuctionHouse.dbc
BankBagSlotPrices.dbc
BattlemasterList.dbc
BarberShopStyle.dbc
CharStartOutfit.dbc
CharTitles.dbc
ChatChannels.dbc
ChrClasses.dbc
ChrRaces.dbc
CinematicCamera.dbc
CinematicSequences.dbc
CreatureDisplayInfo.dbc
CreatureDisplayInfoExtra.dbc
CreatureModelData.dbc
CreatureFamily.dbc
CreatureSpellData.dbc
CreatureType.dbc
CurrencyTypes.dbc
DestructibleModelData.dbc
DurabilityCosts.dbc
DurabilityQuality.dbc
Emotes.dbc
EmotesText.dbc
Faction.dbc
FactionTemplate.dbc
GameObjectDisplayInfo.dbc
GemProperties.dbc
GMSurveyAnswers.dbc
GMSurveyCurrentSurvey.dbc
GMSurveyQuestions.dbc
GMSurveySurveys.dbc
GMTicketCategory.dbc
GlyphProperties.dbc
GlyphSlot.dbc
gtBarberShopCostBase.dbc
gtCombatRatings.dbc
gtChanceToMeleeCritBase.dbc
gtChanceToMeleeCrit.dbc
gtChanceToSpellCritBase.dbc
gtChanceToSpellCrit.dbc
gtOCTClassCombatRatingScalar.dbc
gtOCTRegenHP.dbc
gtNPCManaCostScaler.dbc
gtRegenHPPerSpt.dbc
gtRegenMPPerSpt.dbc
Holidays.dbc
Item.dbc
ItemBagFamily.dbc
ItemClass.dbc
ItemExtendedCost.dbc
ItemLimitCategory.dbc
ItemRandomProperties.dbc
ItemRandomSuffix.dbc
ItemSet.dbc
Light.dbc
LiquidType.dbc
Lock.dbc
MailTemplate.dbc
Map.dbc
MapDifficulty.dbc
Movie.dbc
OverrideSpellData.dbc
QuestFactionReward.dbc
QuestSort.dbc
QuestXP.dbc
PowerDisplay.dbc
PvpDifficulty.dbc
RandPropPoints.dbc
ScalingStatDistribution.dbc
ScalingStatValues.dbc
SkillLine.dbc
SkillLineAbility.dbc
SkillRaceClassInfo.dbc
SkillTiers.dbc
SoundEntries.dbc
SpellCastTimes.dbc
SpellDuration.dbc
SpellDifficulty.dbc
SpellFocusObject.dbc
SpellItemEnchantment.dbc
SpellItemEnchantmentCondition.dbc
SpellRadius.dbc
SpellRange.dbc
SpellRuneCost.dbc
SpellShapeshiftForm.dbc
SpellVisual.dbc
StableSlotPrices.dbc
SummonProperties.dbc
Talent.dbc
TalentTab.dbc
TaxiNodes.dbc
TaxiPath.dbc
TaxiPathNode.dbc
TeamContributionPoints.dbc
TransportAnimation.dbc
TransportRotation.dbc
TotemCategory.dbc
Vehicle.dbc
VehicleSeat.dbc
WorldMapArea.dbc
WMOAreaTable.dbc
WorldMapOverlay.dbc

TBC: 76files 9Mo

AreaTable.dbc
AreaTrigger.dbc
AuctionHouse.dbc
BankBagSlotPrices.dbc
BattlemasterList.dbc
CharStartOutfit.dbc
CharTitles.dbc
ChatChannels.dbc
ChrClasses.dbc
ChrRaces.dbc
CinematicCamera.dbc
CinematicSequences.dbc
CreatureDisplayInfo.dbc
CreatureDisplayInfoExtra.dbc
CreatureFamily.dbc
CreatureModelData.dbc
CreatureSpellData.dbc
CreatureType.dbc
DurabilityCosts.dbc
DurabilityQuality.dbc
Emotes.dbc
EmotesText.dbc
FactionTemplate.dbc
GameObjectDisplayInfo.dbc
GemProperties.dbc
GMSurveyCurrentSurvey.dbc
GMSurveyQuestions.dbc
GMSurveySurveys.dbc
GMTicketCategory.dbc
gtCombatRatings.dbc
gtChanceToMeleeCritBase.dbc
gtChanceToMeleeCrit.dbc
gtChanceToSpellCritBase.dbc
gtChanceToSpellCrit.dbc
gtOCTRegenHP.dbc
gtNPCManaCostScaler.dbc
gtRegenHPPerSpt.dbc
gtRegenMPPerSpt.dbc
Item.dbc
ItemBagFamily.dbc
ItemClass.dbc
ItemExtendedCost.dbc
ItemRandomProperties.dbc
ItemRandomSuffix.dbc
ItemSet.dbc
LiquidType.dbc
Lock.dbc
MailTemplate.dbc
Map.dbc
QuestSort.dbc
RandPropPoints.dbc
SkillLine.dbc
SkillLineAbility.dbc
SkillRaceClassInfo.dbc
SkillTiers.dbc
SoundEntries.dbc
SpellCastTimes.dbc
SpellDuration.dbc
SpellFocusObject.dbc
SpellItemEnchantment.dbc
SpellItemEnchantmentCondition.dbc
SpellRadius.dbc
SpellRange.dbc
SpellShapeshiftForm.dbc
SpellVisual.dbc
StableSlotPrices.dbc
SummonProperties.dbc
Talent.dbc
TalentTab.dbc
TaxiNodes.dbc
TaxiPath.dbc
TaxiPathNode.dbc
TotemCategory.dbc
WorldMapArea.dbc
WMOAreaTable.dbc

Classic 68 files 5.25Mo

AreaTable.dbc
AreaTrigger.dbc
AuctionHouse.dbc
BankBagSlotPrices.dbc
CharStartOutfit.dbc
ChatChannels.dbc
ChrClasses.dbc
ChrRaces.dbc
CinematicCamera.dbc
CinematicSequences.dbc
CreatureDisplayInfo.dbc
CreatureDisplayInfoExtra.dbc
CreatureFamily.dbc
CreatureModelData.dbc
CreatureSpellData.dbc
CreatureType.dbc
DurabilityCosts.dbc
DurabilityQuality.dbc
Emotes.dbc
EmotesText.dbc
Faction.dbc
FactionTemplate.dbc
GameObjectDisplayInfo.dbc
GMSurveyCurrentSurvey.dbc
GMSurveyQuestions.dbc
GMSurveySurveys.dbc
GMTicketCategory.dbc
ItemBagFamily.dbc
ItemClass.dbc
ItemRandomProperties.dbc
ItemSet.dbc
LiquidType.dbc
Lock.dbc
MailTemplate.dbc
Map.dbc
QuestSort.dbc
SkillLine.dbc
SkillLineAbility.dbc
SkillRaceClassInfo.dbc
SkillTiers.dbc
SoundEntries.dbc
SpellCastTimes.dbc
SpellDuration.dbc
SpellFocusObject.dbc
SpellItemEnchantment.dbc
SpellRadius.dbc
SpellRange.dbc
SpellShapeshiftForm.dbc
StableSlotPrices.dbc
Talent.dbc
TalentTab.dbc
TaxiNodes.dbc
TaxiPath.dbc
TaxiPathNode.dbc
TransportAnimation.dbc
WorldMapArea.dbc
WMOAreaTable.dbc
WorldSafeLocs.dbc
killerwife commented 3 years ago

I converted some already, for example Faction.dbc and some others. I have a tool written for this already but its an arduous task.

https://github.com/killerwife/cmangos-tools/tree/master/SpellDBCToSQL

this is the tool I used for everything, it supports loading vanilla, tbc and wotlk DBCs. However if you do this, export ALL columns, not just the ones we know meaning of. Sadly I will not be helping with this as I have my hands full right now.

al3xc1985 commented 4 months ago

Don't we need a new DB created like cmangos_dbc for all of this? The ideea it's nice and open a world of infinite possibilities

Anyways any thoughts in this direction?

MantisLord commented 4 months ago

@cyberium summarized it nicely already:

  • Migrate all sql folder to db repository and change installscript to handle new structure. will help a lot non devs users, only drawback is core devs have to change db scheme from time to time
  • Convert dbc files to sql and put them in db repository. Not really needed and can slow down server start. But make it easier if we want change something.

So there are two different things we're discussing. I feel the DBC to SQL conversion is a bit out of scope - that can be addressed separately if there's any desire. Perhaps we should focus on the core sql folder to DB migration here in this issue.