-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
feat(Core): Allow localized motd #20542
Conversation
New table implementations need to be designed in order to comply with database normalization rules https://www.w3schools.in/dbms/database-normalization |
Quick copilot review: The provided SQL script modifies the
To ensure compliance with database normalization rules, especially the Third Normal Form (3NF), the following considerations should be made:
Recommendations
This structure will ensure that the database is normalized and follows best practices for managing localized content. |
@Nyeriah , any suggestions regarding the commands affected by this PR or the OnMotdChange() script |
Yes, they will crash if used from console. Handler in the command context is not guaranteed to be a player or have a valid session, so this needs to be checked |
Thank you for this insight. My thoughts would be:
|
Sure, it can be added as an optional, and it should default to english |
Please keep the MOTD encoding, it's an AC thing to difficult it being removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go if you address these small things
Code adjusted. Should be good to go. |
Did you test this after the changes were made? If so we can merge |
Yes, its tested and working. I had to fix the server command, but now everything is good to go. |
Awesome. We can merge after DB Squash. Great job :D |
CI failing |
Already on it ;) |
Are you in the discord? |
Yes. I am the japanese named avatar you responded earlier with the spell dbc in the dev-core channel ;) |
I see I see |
* Initial commit for localized motd * Rename function that created world packages * Update to satisfy code check * Update code to accomodate localized motd * Update command to support multiple optionales & adjusted db * Code cleanup * Update sql name * Fix codestyle issues * Remove hardcoded schema * Add check for valid player in reload command * Update to better code style * Add missing include * Fix redundant code usage * Add missing include * Remove sql files and create new rev sql files * Address minor code reviews * Fix code style * Update code to address code revisions. - Remove two unused functions - Remove map - Use available function to resolve LocaleConstant * Fix code style * Add check for base motd and update locale to DEFAULT_LOCALE * Code docs * Removed some docs, readd defaultd motd formatting * Fix oversight in variable declaration * Code style fix * Update code based on code review * ready for merge * Fix set motd command due to changes to DEFAULT_LOCALE * Fix CI * Fix trailing whitespace --------- Co-authored-by: Kitzunu <[email protected]>
Changes Proposed:
This PR proposes changes to:
Issues Addressed:
SOURCE:
The changes have been validated through:
Tests Performed:
This PR has been:
How to Test the Changes:
Known Issues and TODO List:
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.