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

feat: offline generation of action packs #3995

Open
4 tasks done
johnnylam88 opened this issue Oct 20, 2024 · 9 comments
Open
4 tasks done

feat: offline generation of action packs #3995

johnnylam88 opened this issue Oct 20, 2024 · 9 comments
Assignees
Labels
backlog Low priority; awaiting time/resource availability. enhancement

Comments

@johnnylam88
Copy link
Contributor

Before You Begin

  • I confirm that I have downloaded the latest version of the addon.
  • I am not playing on a private server.
  • I checked for an existing, open ticket for this request and was not able to find one.
  • I edited the title of this feature request (above) so that it describes the issue I am reporting.

Feature Request

I'd like to see the ability to generate action packs from priorities without needing the WoW client to be open. This helps improve maintenance of the addon and with easing updates to the latest SimC priorities.

Additional Information

I think the following steps need to be taken:

  • Move the action pack description into a header comment in the associated priority file. This makes the priority file hold all of the information in the action pack.
  • Move the actual action packs from the various spec files into a separate directory, e.g., TheWarWithin/ActionPacks/DeathKnightBlood.lua, etc. This is tremendously helpful for minimizing diffs to the actual spec files as the actual action pack lines are just generated noise in the diffs.
  • Separate the importer and exporter into a standalone module.
  • Write a linter that can strictly verify the syntax of a priority and also eventually check that all abilities and auras named in the priority are defined.
  • Write a Lua script that can generate a Lua file containing an action pack for an input file with the corresponding priority.

Contact Information

No response

@johnnylam88 johnnylam88 added enhancement triage Issue needs initial triage to determine if action needs to be taken. labels Oct 20, 2024
@johnnylam88
Copy link
Contributor Author

I'm opening this issue as a discussion point. I think this will be very useful for the project. I'm happy to devote a lot of time to submitting patches for review and getting this done.

@Hekili
Copy link
Owner

Hekili commented Oct 21, 2024

I've debated going this route for quite a while.

Instead of ActionPacks, the folder should be Priorities.

Honestly, the shortest implementation of this just requires:

  • Priority Name
  • Priority Specialization
  • Priority Version #
  • Priority Description / Notes
  • SimC APL as a long string [[...]]

Then, the work of RestoreDefaults in Classes.lua can just do the importing if (1) there isn't an existing priority, (2) the existing priority is missing version info, or (3) the existing priority's version # (date) is less than the file's version.

That doesn't solve for offline serialization, but we could still do validation of the priority itself later.

@Hekili Hekili added backlog Low priority; awaiting time/resource availability. and removed triage Issue needs initial triage to determine if action needs to be taken. labels Oct 21, 2024
@johnnylam88
Copy link
Contributor Author

That works, too. The import does cause extra loading time, so people will see longer loading screens as a result, but it has the benefit of simplicity, and not needing to synchronize between the APL and the action pack string.

@johnnylam88
Copy link
Contributor Author

We can make header comments in each Priority/<spec>.simc file that hold all of the information for a single action pack in a structured comment, then parse that header comment when auto-importing.

@johnnylam88
Copy link
Contributor Author

I'll work on this for a pull request, and then a later one as well for a linter to verify the priority file.

@Hekili
Copy link
Owner

Hekili commented Oct 21, 2024

As far as I know, we can only load .lua files, but your point stands. We can load them as .lua files.

@johnnylam88
Copy link
Contributor Author

Ah, yes, not sure what I was thinking there. We can only load Lua files, so the SimC APL would just be a string inside the Lua file, much like the current action pack, so the rest of the bits would just be key/value pairs inside a table that gets registered with the addon.

@johnnylam88
Copy link
Contributor Author

johnnylam88 commented Nov 1, 2024

I'm currently working on this and I see in Classes.lua an empty function:

function spec:RegisterPriority( name, version, notes, priority ) end

I believe that this function was a placeholder to do what you (@Hekili) had described above. I'll implement the body of this function, use it to create default priorities in TheWarWithin/Priorities/<Classs><Spec>.lua and then modify RestoreDefaults to do the auto-import.

@johnnylam88
Copy link
Contributor Author

I'm going to split the progress on this into separate pull requests. The first will just contain the new code plus the changes to RestoreDefaults and try to load the priority first but to fall back to an existing pack. The second will be to move the existing packs into Priorities/<class><spec>.lua files. The last will be to remove all of the existing pack strings.

In this way, the changes are incremental and won't break anything (hopefully!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Low priority; awaiting time/resource availability. enhancement
Projects
None yet
Development

No branches or pull requests

2 participants