Revamp "AnimationContext" Docs, now called "AnimTaskQueue"

[fig02]

I got sent down a rabbit hole while doing Player docs which required a better understanding of what we used to call "AnimationContext".

After learning about it, I decided the current names for basically all of this system could be improved. After a few iterations I ended up calling it the "AnimTaskQueue" and changed a bunch of terminology throughout.
It sounds quite generic, but it kinda has to be. The various tasks aren't very correlated, other than the fact that they are related to animations.

All of the relevant functions in skelanime have comments at the top that hopefully explain everything, but let me know if anything else needs to be clarified.

[cadmic]

group makes it sound like this is an id, but I was surprised to find that this is a bitset. Maybe groups or groupFlags?

[fig02]

not sure if this became clear as you read more of the code, but its always going to be just one group number. it is handled with bits yes, but this value within each task will never hold multiple groups/bit numbers. it is a single group number that increments as "1, 2, 4, 8..." instead of "1, 2, 3..."

[cadmic]

Hmm I still think conceptually the group ids are 0, 1, 2, ... and that this field really represents 1 << group rather than the group id itself. Maybe a comment to that effect?

[fig02]

you can think of them as "regular" incrementing number sure, but the system never operates on it that way. The current group number is initialized with 1 (bit position 0) and incremented with <<= 1.
I would feel different if there was some interface with which a user can pass a group number as a "whole number" and the system does the shifting interpretation on its end. but this isnt the case.

It could be called a groupBit or something, but this is being more obtuse than needed imo, its still a single number that identifies a group, even if it doesnt count up normally.

[cadmic]

Hmm I think sDisableAnimQueueFlags/sDisabledTransformTaskGroups is important here. It doesn't really make sense to or-together the different group ids; normally the only thing you're allowed to do with ids is compare them for equality. And the if (!(task->group & sDisabledTransformTaskGroups)) is also pretty mysterious.

But it becomes clearer if you think of sDisabledTransformTaskGroups as a bitset where each bit represents a group i.e. group 0 is represented by bit 0, group 1 is represented by bit 1, and so on. Then group and sCurAnimTaskGroup are also group bitsets with one element set, or (1 << group_num) where group_num is the "real" group number (although it's never actually written down anywhere), and so if (!(task->group & sDisabledTransformTaskGroups)) is testing whether the group number is contained in the set of disabled groups.

In summary: I still think this and sCurAnimTaskGroup should be thought of as (1 << group_num), rather than single number that identifies a group, based on how sDisabledTransformTaskGroups is used. I think the names and comments should hint at that. Maybe group and sCurAnimTaskGroup is fine and my issue is more with the comment wording, the comments seem to treat this bit-twiddling as some kind of arcane implementation detail but it becomes easier to explain if you treat group numbers as bit numbers rather than powers of 2.

[mzxrules]

ignoring the id value aspect thing for a moment, I don't understand what's being "grouped"

[fig02]

Different tasks or "entries" are being grouped together. Then, when AnimTaskQueue_DisableTransformTasksForGroup is called, the "transformative tasks" only for the currently active group (at the time of calling) will be skipped for that frame.

In practice, there are only two "groups" that get used. all of Links anim tasks are in one group, and all other actors tasks are in another. This lets Link disable the transformative tasks for his stuff, without affecting others.
The same could be done in reverse, but AnimTaskQueue_DisableTransformTasksForGroup is only called by player.

[cadmic]

I'd prefer this comment to go on sCurAnimTaskGroup instead (something like that this is a bitset where exactly one bit it set for the current group)

[fig02]

If I was learning about this system for the first time I personally would first look at functions that interface with the system, rather than the piece of data itself. Maybe its worth commenting on both idk

[cadmic]

I tend to look at the data first. As the saying goes: "Show me your flowcharts and conceal your tables, and I shall continue to be mystified. Show me your tables, and I won’t usually need your flowcharts; they’ll be obvious."

[fig02]

"Group numbers are indicated by a single bit that increments by shifting 1 position to the left. See AnimTaskQueue_SetNextGroup for more details"

would you be fine with this comment on the data member?

[cadmic]

Maybe something general about group numbers would be helpful, but for the data members I'd prefer the comment to be more precise about what the data member itself represents, e.g.

// A bitset where the `i`th bit is set if the `i`th task group is disabled.
static u32 sDisabledTransformTaskGroups = 0;
// `(1 << i)` where `i` is the current task group.
static u32 sCurAnimTaskGroup;

Armed with this knowledge, the reader can guess that sCurAnimTaskGroup <<= 1 is conceptually incrementing the "current task group", and something like sCurAnimTaskGroup & sDisabledTransformTaskGroups is testing whether the "current task group" is "disabled".