Azeroth-Pilot-Reloaded / azeroth-pilot-reloaded

Azeroth Pilot Reloaded
GNU General Public License v3.0
29 stars 10 forks source link

added map with classes and their numbers to improve readability #151

Closed julien-lanctot closed 1 year ago

julien-lanctot commented 1 year ago

finished refactoring classes numbers as names

That way we can easily tell that 1 = warrior and so on, just by looking at code

NightofStarrs commented 1 year ago

I would prefer if we moved this into the existing APR.Class dictionary, this is already being done with APR.Race. This way we are not merely creating a new dictionary and file, and instead being used with an existing dictionary that can be updated.

Neogeekmo commented 1 year ago

I would prefer if we moved this into the existing APR.Class dictionary, this is already being done with APR.Race. This way we are not merely creating a new dictionary and file, and instead being used with an existing dictionary that can be updated.

I prefer to rename APR.class to APR.className, APR.classFilename, APR.classId, because class[1-3] doesn't mean much either...

julien-lanctot commented 1 year ago

@NightofStarrs I dont think its a good idea to have the enum of classes, races, etc in the same file where we defined the global APR variables. Its best to separate types in individual files to not bundle everything together. Also you mention the APR.Race dictionnary but i cannot find it? All references of APR.Race in the code just check if APR.Race = some string that references the race.

@Neogeekmo I agree, i generally think we should get rid of magic values, espicially for integer used as boolean (unless theres a specific reason why its done that way.

Overall, the best approach would be to have every "option" for race, sex, classes, etc in separate files that we can update for future expansions, to reduce the size of Core.lua, which is already 3k + lines long and loaded with functions that could be separated as well.

Neogeekmo commented 1 year ago

Merged to avoid conflicts with V3