Skip to content

Conversation

@Cidan
Copy link
Contributor

@Cidan Cidan commented Aug 21, 2022

This CL resolves #646 and fully unifies the codebase for all three versions of World of Warcraft on the market. It works by adding 4 new constants to Constants.lua that can determine the running version at runtime. These constants are then used to conditionally branch various parts of the code so AdiBags works across all three releases from a single codebase.

This CL also adds support for building all three versions via BigWigsMods packager under a single git tag. At any point after this CL, simply upload a new tag and the Github action tied to this repo will build the correct artifacts for each version of WoW.

Things to note:

  • Some parts of the code that are "dead" and unused are not conditionally escaped for classic. This is intentional and harmless, as the code is never actually run, and there's no use in escaping it
  • Only code that is actively executed by WoW and needs to be changed has been modified
  • The config screen still needs a few changes for classic, i.e. there are still references to glyphs
  • This has not been tested with the coming WotLK release, but will be addressed when pre-patch is released with the new API's

I've tested on retail, classic, and classic era:

  • Bags
  • Filters
  • Bank Bags
  • Reagent Bags
  • Void Storage

I've encountered no obvious bugs. @Talyrius Can you do a quick lookover and see if this looks good? I know you're not around much, which is fine -- if I don't hear from you by sometime next week, I'll go ahead and merge this.

Thanks!

@Cidan Cidan requested a review from Talyrius August 21, 2022 19:11
@Talyrius Talyrius mentioned this pull request Aug 21, 2022
@Cidan
Copy link
Contributor Author

Cidan commented Aug 21, 2022

If you have no comments, I'm going to merge this so I can get started on the current open issues. Unrelated, but thanks with the help triaging the older issues that needed to be resurfaced @Talyrius :)

Copy link
Contributor

@Talyrius Talyrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than some whitespace nitpicks (unimportant as the rest of the codebase is inconsistent as it is), nothing stands out to me. I haven't yet reactivated my sub to test drive it, but you said you had already done so anyway. Looks good! 👍

@Cidan
Copy link
Contributor Author

Cidan commented Aug 22, 2022

Awesome, I'll merge this later tonight!

re: Whitespace, I'm thinking about adding some linting rules to fix this in general. I'll revisit it.

@Cidan Cidan merged commit 1e36f59 into master Aug 22, 2022
@Cidan Cidan deleted the unified-build branch August 22, 2022 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Properly create TBC/WotLK releases based on BigWigsMods release mechanisms

2 participants