Playerbots AC-fork cleanup - #238
Draft
NoxMax wants to merge 14 commits into
Draft
Conversation
There's a lot in these files, so doing them on their own.
Also define a magic number. Keep OnPlayerIsClass in the graveyard death knight check Drop the DispelType underlying type. Nothing uses it.
The only caller set the mask immediately after construction, which the existing defaultUpdateMask parameter already does. Lets _updateFlags stay const.
Also fixes issue with bot guild tabards
It was flagged as a database-validation candidate, but PlayerbotScript is never assigned by name in the database, so CheckIfScriptsInDatabaseExist had no reason to search it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
The vast majority of this cleanup is format based, but I found a number of changes in the Playerbots fork of AC that can be done in less code, or with code outright move onto the Playerbots repo itself. Which brings me to the point that this requires the merger of mod-playerbots/mod-playerbots#2694 over there. About
core-build.yml, its whole job is done bycore-build-playerbots.yml, so it's redundant.Compiles and runs on Linux of course, and did some basic validation with BGs and so far things seem to be stable. But there needs to be more validation that I would like other reviewers to take a look.
This is the current diff of AC test-staging with master, vs the diff of this PR with master:
Drafting until other people take a look for some finalizing polish.
Feature Evaluation
How to Test the Changes
Impact Assessment
Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots?
Does this change modify default bot behavior?
Does this change add new decision branches or increase maintenance complexity?
AI Assistance
Was AI assistance used while working on this change?
Did a first pass myself of things that can be cleaned, then had claude go over to suggest any other cleaning opportunities and review. In the process there were few things that I found that need their own fixes, but they are global fixes that should be done upstream on AC.
Code Provenance / Attribution
Was any code in this PR copied or adapted from a sister / upstream project (e.g. CMaNGOS playerbots,
MaNGOS, another module)?
Final Checklist
Notes for Reviewers
Changes in this PR don't include #237, which drops the diff even more.