Skill System v2.0 proposal

[sme23]

This is a sort of all-in-one proposal for making & keeping the project clean and using uniform conventions, the direction of the project moving forward, and a solution to foreseen long-term issues; this issue also obsoletes what remains of issue #58.

The current version of the Skill System, 1.2, has been plagued with a series of issues for a long time: a significant amount of stuff included is not a part of the Skill System itself, a lot of the older parts use discouraged practices in their implementations, and a focus on remaining backward-compatible prevents a lot of these things from being fixed, only accounted for. Per semantic versioning rules, a 2.0 version would allow for dropping the consideration for backward compatibility, opening the door for cutting a large amount of the fat out of the project and keeping it focused on just the core system.

Looking forward to a setting where the FE8 decompilation is completed, there becomes an issue that used to also be an issue with assembly hacks, namely installing multiple of them without having to manually integrate pieces of each. The two solutions to this that we have now for assembly hacks are EA installers, allowing them to be installed to and reference wherever there is free space for them in the ROM automatically, and modular hooks, allowing multiple hacks to hook in the same locations without requiring the end user to manually integrate them beyond writing a label name in a specific location. Building directly from source code inherently solves the first of these issues, but the second of these returns: any two modifications that edit the same function(s) require manual integration to use together.

Given all of this, the proposal is as follows:

• Remove all superfluous hacks and skills that rely upon them
  • This constitutes everything in the ExternalHacks and QualityOfLife folders. There should be 3 skills that integrate with an external hack, which would be removed along with them.
• Rewrite all core systems in C
  • This is the contents of Necessary and SkillSystem/Internals, much of which is already in C, but the rest needs to be converted still.
• Replace all inline edits
  • All existing hooks and inline edits are to instead target full function replacements using lyn autohooks. The combination of this and the previous point makes for a decomp-compatible framework for dealing with conflicting changes to the same function(s).
• Rewrite all skill functions and hooks in C
  • This constitutes converting all skill-specific hooks into calculation loops called from full function replacements, as with the rest of the inline edits.
• Standardize code styling & practices
  • The styling should be able to be done automatically with a linter
  • Standardized practices are light, mostly just utilizing new data structures instead of editing existing ones for compatibility purposes
• Document everything!!!
  • Utilize the wiki to document every hook and how it works, the arguments of the functions each calls, the layout of new data structures, the core system and how to utilize it, adding skills, etc. Anyone should be able to edit the wiki, but it has to be done separately from pull requests.
• Stage 2.0 on its own branch
  • The existing master branch would become v1.2.x and remain the default branch for the repository until 2.0 is ready for a release, and 2.0 work would be staged on a new v2.x branch. This allows the existing version to remain as the default and recommended one until 2.0 is ready, and remain on its own branch afterward to receive maintenance for bugfixes and such as needed.

This should solve longstanding existing organizational and bloat issues, give a platform moving forward to address future issues from, and deal with the technical debt left by some of the older components still used in the skill system. I know various parts of this are things that other people have started working on before and had nowhere to put incomplete work towards, so hopefully this setup would allow for a place to put incremental work towards some of these goals. #

Before any changes are actually made I want people's thoughts on this proposal, preferably as replies to this GitHub issue to keep them all organized in one place.

[JesterWizard]

As someone who contributes regularly to the skill system, I wholly support a more concentrated focus on documentation and consistent coding practices. It's a bit of a wild west at the moment, especially between the older and newer hacks. Finding out how to do specific things right now often boils down to hopping between FEuniverse topics and the gba coding channel on Discord. Having a centralised location for this information is best.

Removing a lot of the bloat in the system is also a change I greatly welcome. I can't speak for others but I don't find myself using those hacks that often, and feels somewhat misleading to bundle dozens of features that have little to do with the system. Moving them into their own little project feels appropriate.

I think the move to a C system is best. Mokha's system has opened my eyes to how much easier contributions become in a higher level language, and I say this as someone who has contributed here exclusively in ASM. The decomp has gotten to the point where it should really be the focus going forward, with perhaps a legacy system maintained for people who enjoy contributing their hacks in ASM (the three of us anyway).

Removing skill specific hooks and introducing calc loops for everything sounds good, but I'm not sure of the feasibility of doing it for everything, outside of hacks like Vantage and Armsthrift that could clearly benefit from it. But if it enables people to more easily create variations of these skills to fit their own needs, then I'm all for it.

All in all I support this proposal, but I'd be interested to hear the opinions of others who can provide a more technical weighing of the pros and cons.

[CT075]

I think you should probably decide, right now, what the intended workflow will be for extension authors. With the old world, the method to add a new skill was "locate the correct calcloop, figure out which skills to sandwich your new functionality between, manually write a function in assembly adjusting the attacker/defender struct directly".

Having a concrete story in mind for "how do I add a new skill" is, in my mind, the single most important feature of a base skill system.

[Veslyquix]

I'm not against external hacks existing long term (for example, strmag split feels like it's an integral part of skillsys), but I do think for a c based or skillsys "2.0" means starting from very little. I don't really want us to split our efforts between two similar projects, but Mokha has made some impressive progress towards a c based skill system, so I do wonder if it might make more sense to jump ship to that as a starting point. Additionally, Mokha seems to want to support use through febuilder which is something I'm aligned with. I don't think that everything needs to necessarily be in C, but I do think that most core hacks should be in C or at least have good documentation.