Make IDs easier to edit

[dedmen]

In case you need to edit the Entity/GameObject ID's it can now be done in just 4 places.
Instead of in two dozen

• Closes CreatureTemplate/GameObjectTemplate ID's are partially hardcoded #44

[Kitzunu]

Okay, but what if I set @C_TEMPLATE = 40050, then everything would break.

[dedmen]

Same as before.
But now you can easily change it in this one (four) place(s).
If you change it in SQL script, you need to change it here too.

But atleast there are only 4 places to change, instead of two dozen as before

[Kitzunu]

well then put it into the config. Hardcoded stuff are bad.

[pangolp]

ERROR: Permission to DedmenWoW/mod-guildhouse.git denied to pangolp.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I tried to make modifications and solve the conflicts, but when it comes to make the push, I find that I don't have permissions to upload the changes. If you could enable those permissions, I would push the changes. @dedmen

[dedmen]

Huh weird the checkbox isn't present to allow access to maintainers.
I've sent you invite to my project to give you access. Thank you

I just checked for changes here 3 days ago :D

[pangolp]

Huh weird the checkbox isn't present to allow access to maintainers.
I've sent you invite to my project to give you access. Thank you

I just checked for changes here 3 days ago :D

From the mobile phone, when I want to open the invitation, it gives me a 404 error. However, I will try from the PC. It's strange, usually, they don't have to invite me. Simply modifying the permissions in the fork settings is usually enough.

I have to check if, in addition to solving the conflict, I don't have to make one more change. Because I was not sure if when editing a SQL file, a new hash was created and executed again. Yesterday I confirmed that, and I saw that it does. But without knowing it, I created a third file. After I upload the changes, I will have to test the branch and verify that everything is fine.

[pangolp]

The conflict is resolved, but I think some suggestions about codes or new hardcore are missing.

[pangolp]

I think the changes are compatible, but we still need to review the hardcore numbers part.

[dedmen]

The hardcoded numbers will also get worse when we add SQL patches, like we did with the previous PR you merged.
The update script again has a hardcoded number in it, which again needs to be changed.

Maybe we could store the "base number" in the database, some table.
Then c++ code can fetch it from there, SQL update scripts can fetch the base number from there, and we can make it changeable in one single place (the initial init SQL script)

But that's a quite bigger change. I didn't want to provide a full solution here, I just wanted to make it a bit easier (edit 4 places instead of over 20), and the better solution can be done later (more easily because I already did half of it).

Also the way I did it with the constexpr function, means the value needs to be present at compile-time, so putting it into config file isn't possible that way. But it solved my use case as I compile from source with edited ID's (which you already had to do previously)

[pangolp]

The hardcoded numbers will also get worse when we add SQL patches, like we did with the previous PR you merged. The update script again has a hardcoded number in it, which again needs to be changed.

Maybe we could store the "base number" in the database, some table. Then c++ code can fetch it from there, SQL update scripts can fetch the base number from there, and we can make it changeable in one single place (the initial init SQL script)

But that's a quite bigger change. I didn't want to provide a full solution here, I just wanted to make it a bit easier (edit 4 places instead of over 20), and the better solution can be done later (more easily because I already did half of it).

Also the way I did it with the constexpr function, means the value needs to be present at compile-time, so putting it into config file isn't possible that way. But it solved my use case as I compile from source with edited ID's (which you already had to do previously)

If you can't edit it, from the configuration file, then bye-bye to the solution I wanted to give you...
What happens if we remove the constexpr and add the value in the configuration file?

[dedmen]

If you remove constexpr it fails to compile.
Because the switch cases need to be compiletime constant values https://github.com/azerothcore/mod-guildhouse/pull/45/files#diff-718379843cd95689dd90c3b83f7107660e3c4bdaa7d3930617e3239dfac24f2eR259-R266

Of course can make that not be a switch and instead some different runtime dispatch. But that would be a bigger rework there too that I didn't want to go down to.

Maybe I'd have some time soon-ish to do just that. But probably would be a couple weeks to get to it.

[pangolp]

If you remove constexpr it fails to compile. Because the switch cases need to be compiletime constant values https://github.com/azerothcore/mod-guildhouse/pull/45/files#diff-718379843cd95689dd90c3b83f7107660e3c4bdaa7d3930617e3239dfac24f2eR259-R266

Of course can make that not be a switch and instead some different runtime dispatch. But that would be a bigger rework there too that I didn't want to go down to.

Maybe I'd have some time soon-ish to do just that. But probably would be a couple weeks to get to it.

I leave you the updated branch then, the module compiles and works, when you have time to check it, welcome.

[pangolp]

What this person did, doesn't it work, or is it complementary to what you want to do: #39.

[dedmen]

#39 is completely unrelated to my thing.

[Helias]

if no objections, I would merge this, it improves the code quality