search

cmangos / issues

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

Creature Text Makeover #2331

Open MantisLord opened 3 years ago

MantisLord commented 3 years ago

Current State

We have many different tables which may contain raw text used by creatures/npcs (either during script, combat, or gossip).

I'll list them all here for convenience:

Now, we also have a newer, more recently discovered addition to this list: broadcast_text

Overall this means we actually have a significant amount of duplication. All of the above usages may instead be tied to a corresponding broadcast_text id.

What is broadcast text, and why do we love it?

Broadcast text contains the "truest" source of information we'll probably ever have for all of this data.

What we have now is a combination of data that was gathered on retail with the MoP+ client and on Classic. Data from 1.13 classic was prioritized over the data from MoP because it's "closer" to the era we're targeting here with CMaNGOS (Vanilla-WotLK). At a certain point though, the 1.13 broadcast text IDs stop and we need to rely on the MoP+ data for TBC and WotLK. Broadcast text locales also contains the official translations for all of the texts. This is why broadcast text ids should always be used when scripting creatures.

99% of the old broadcast text entries remained untouched as new expansions were released. It's very rare that old entries were retroactively edited, although we have found a few cases. See here for an example of a text which should change over each expansion, but maintain the same broadcast text ID: https://github.com/cmangos/tbc-db/commit/f6abac3ad98e2a9d826b44c4e94b7bfd974d085e UPDATE - these special text adjustments are now done directly to broadcast_text in this file: https://github.com/cmangos/tbc-db/blob/master/utilities/cmangos_custom.sql

So we have 100% certainty when it comes to the Vanilla (thanks to Classic 1.13) and WotLK (3.3.5+) versions of broadcast text ID 8106, since both were gathered from official sources. However the TBC version remains a mystery and can only be speculated about at this time. Once B decides to relaunch TBC in the near future, we will likely be able to get an updated source of broadcast text for TBC, but overall there will not be many changes over what we have now. UPDATE2 - With the release of Classic TBC 2.5.1, we are able to start making corrections to the texts from Classic 1.13 and MoP+. Example: https://github.com/cmangos/mangos-tbc/commit/17281209469ab70de8dcd5596817d28cda82aa04 (still more updates to come based on this new official 2.5.1 data)

The Cleanup Proposal

Don't duplicate data, and don't use texts are handwritten by someone or copied from wowhead/videos.

Instead point everything to the one source of data - broadcast_text and broadcast_text_locales.

Any time broadcast text lacks a sound or emote referenced by one of our existing tables, add it in. If we find that text must be modified for any reason (such as the TBC-DB update above), we will maintain a separate update file in the core repo SQL folder - similar to how spell DBC data is treated with the Spell.sql file.

Once all scripts are re-pointed to reference broadcast_text.id and it is ensured no data loss will occur, DROP ALL of the tables in my list above (aside from gossip_menu_option which may simply have its option_text/box_text fields removed).

If we decide to go through with this specifically for script_texts and gossip_texts, there will be one huge update to all of our enums in ScriptDev2 which define script_text/gossip_text entries. So I'll end up with quite a few changes across hundreds of SD2 scripts looking very similar to this:

image

Also, keep in mind some additional core systems and files will have to be touched to remove all reference to the old tables once they are made fully redundant. Are there any downsides here I'm not considering? I'm willing to proceed with the operation for all 3 databases/cores assuming no one is concerned over moving away from all of these old tables.

Version & Environment

Classic, TBC, and WotLK

Grz3s commented 3 years ago

+1

Valkirie commented 3 years ago

+1

cala commented 3 years ago

:+1:

I also have in mind for a long time a rework/optimisation/consistency cleanup of many tables in our database, similar to what you are proposing here, I'm simply lacking the time to do so.

Lymphome commented 3 years ago

What is planned with other tables (I don't know if everything is still in use, if all are listed) ?

It would be nice to have different starting IDs for each of these tables (for example, spell template would start at 10M, quest template at 20M, trainer greeting at 30M... and 1G for custom contents), and increment the IDs in relation with source table IDs (for example for spell_template => text id in broadcast_text is 1xxxxxyy, where xxxxx is the spell id and yy is the text id) : easier to add some missing texts, to find from where a text comes, to build sql queries... Just an idea

MantisLord commented 3 years ago

What is planned with other tables (I don't know if everything is still in use, if all are listed) ?

  • spell_template (32 fields) (total : 29039)
  • quest_template (10 fields) (total : 6481)
  • trainer_greeting (1) (total : 661)
  • questgiver_greeting (1) (total : 167)
  • points_of_interest (1) (total : 377)
  • page_text (1) (total : 1431)
  • item (2) (total : 25006)
  • gameobject (3) (total : 14471)
  • creature (3) (total : 18806)
  • areatrigger_teleport (2) (total : 186)

It would be nice to have different starting IDs for each of these tables (for example, spell template would start at 10M, quest template at 20M, trainer greeting at 30M... and 1G for custom contents), and increment the IDs in relation with source table IDs (for example for spell_template => text id in broadcast_text is 1xxxxxyy, where xxxxx is the spell id and yy is the text id) : easier to add some missing texts, to find from where a text comes, to build sql queries... Just an idea

It's an interesting idea you have, but I think we should avoid adding custom IDs to the broadcast_text table if at all possible. Since there is no official broadcast text available for the data in these other tables you listed, we need to correct or add any missing texts in the source tables and their corresponding locales tables. For all the tables you mentioned, there is an equivalent *_locales table already and we should populate those if there is anything missing.

al3xc1985 commented 2 years ago

@killerwife added?

killerwife commented 2 years ago

Not finished on vanilla and wotlk branches

AnonXS commented 2 years ago

This is added now, though more conversions have to be made (SD2 -> BCT etc). I'd say this can be closed. good work @ all.

AnonXS commented 1 year ago