Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating DataStores #13508

Merged
merged 22 commits into from Nov 10, 2014
Merged

Updating DataStores #13508

merged 22 commits into from Nov 10, 2014

Conversation

chemicstry
Copy link
Contributor

I will be updating datastores, one per commit as requested. I'll keep PR open and keep adding changes.

@xerkoss
Copy link
Contributor

xerkoss commented Nov 5, 2014

why u change WorldSafeLocsEntryfmt? I don't have any WorldSafeLocs.dbc file.
i'm not sure if it's properly extracted. Or it doesn't exist.
anybody knows?

@chemicstry
Copy link
Contributor Author

It was a typo from previous old commit. That file was still for 4.3.4 and I haven't looked into it

@chemicstry
Copy link
Contributor Author

Yeah, I know that AreaPOI is now db2 but it wasn't used in core at all. Maybe other devs can remember why it was there.

@xerkoss
Copy link
Contributor

xerkoss commented Nov 6, 2014

DBFilesClientList.h needs to be updated i think. A lot of missing or useless files. I added "DBFilesClient\WorldSafeLocs.dbc", and extracted properly my missing file :).

@chemicstry
Copy link
Contributor Author

There were a lot of discussion on irc about renaming dbc structs to official blizz names (see: http://pastebin.com/vHrJkpQn ). The main problem is that blizzard uses inconsistent names (MapID, InstanceID, ContinentID - all mean the same) and float arrays for coords (instead of xyz). Other than that it's all OK. I would like to hear other developer input on this before I continue my work.

Options are:

  1. Keep old names
  2. Use all blizz names
  3. Use blizz names with modifications regarding inconsistency and coords

ExpansionRequirementContainer const& GetRaceExpansionRequirements() const { return _raceExpansionRequirementStore; }
uint8 const GetRaceExpansionRequirement(uint8 race)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const on value return does not make any sense (and will trigger warnings on certain compilers)
also the method should be made const as whole

uint8 GetRaceExpansionRequirement(uint8 race) const

the same applies to GetClassExpansionRequirement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll fix this soon.

…reatureSpellData.dbc, CreatureType.dbc, CurrencyTypes.dbc, DestructibleModelData.dbc structs
@chemicstry
Copy link
Contributor Author

@xerkoss I will fix custom sql formats after I'm done with files.

@jackpoz
Copy link
Member

jackpoz commented Nov 10, 2014

@streetrat looks like this pr does something similar to b4a9f47

@Funkeln
Copy link

Funkeln commented Nov 10, 2014

What happened to "just few updates per pull request"? Isn't adding all of them to a single pull request going to make this take ages to be reviewed and eventually work start getting overlapped? Just wondering :)

@streetrat
Copy link
Contributor

im fixing build and adding some comments for this, please open a new PR for the new updates.
thank you

@streetrat streetrat merged commit 43d5fb5 into TrinityCore:6.x Nov 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants