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

fix(DB): clarify IDs for faction changes #20480

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

TheSCREWEDSoftware
Copy link
Contributor

@TheSCREWEDSoftware TheSCREWEDSoftware commented Nov 8, 2024

Changes Proposed:

Tables:
player_factionchange_achievement
player_factionchange_quests
player_factionchange_reputations

Are having their tables drop and created to add the new comment column and their respective data.

This PR proposes changes to:

  • Core (units, players, creatures, game systems).
  • Scripts (bosses, spell scripts, creature scripts).
  • Database (SAI, creatures, etc).

Issues Addressed:

  • None to my knowledge

SOURCE:

  • Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
  • Sniffs (remember to share them with the open source community!)
  • Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
  • The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found
  • Other tables have, these don't, rationally makes no sense to not have them.

Tests Performed:

This PR has been:

  • Tested in-game by the author.
  • Tested in-game by other community members/someone else other than the author/has been live on production servers.
  • This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

  • This pull request can be tested by following the reproduction steps provided in the linked issue
  • This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.

Known Issues and TODO List:

  • player_factionchange_achievement
  • player_factionchange_quests
  • player_factionchange_reputations

Added "comments" for the achivements of each faction equivalent.
@github-actions github-actions bot added the DB related to the SQL database label Nov 8, 2024
@TheSCREWEDSoftware TheSCREWEDSoftware marked this pull request as ready for review November 9, 2024 11:17
@Grimdhex
Copy link
Contributor

The PR is ready to be review ?

@TheSCREWEDSoftware
Copy link
Contributor Author

Yes it is

@Grimdhex Grimdhex changed the title fix(DB/player_factionchange_achievement & "quests & "reputations) fix(DB): clarify IDs for faction changes Nov 15, 2024
@sudlud
Copy link
Member

sudlud commented Nov 15, 2024

there was still an ongoing discussion about this in discord iirc.

@Grimdhex Grimdhex added To Be Merged - Later This PR promotes delicate changes and has been flagged to be merged after the holiday seasons. and removed To Be Merged labels Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB related to the SQL database To Be Merged - Later This PR promotes delicate changes and has been flagged to be merged after the holiday seasons.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants