oot
oot copied to clipboard
zeldaret
Drop table tables and `DROPFLAGS`
Realised that this was staring us in the face as something to do the whole time. I don't love how they turned out, but they're way more readable than the previous table. The _ROW macros are essentially transparent, but good for the groupings that we think of in the drop tables; we might consider whether or not they should have commas, and if the numbering should be in hex or dec. I used _ROW because I also wanted to enumerate them for calling the Item_DropCollectibleRandom uses. It seems correct to use dec for the actual row numbers, but since the actual indices are only accessed in Item_DropCollectibleRandom, I went with hex since that's what we usually do.
There is one significant wrinkle, namely the enigma of EnWonderItem, which seems to have scant regard for how this system works, and very much seems to be doing its own thing. I did not attempt to fit it in in this PR, it has enough special cases and complexity that it would take more time to sort out than the rest of this put together.
Added a set of flags for the Actor.dropFlag variable. As usual almost everything just accesses them through the two functions designed for that purpose, with the usual exception of Deku Babas doing their own thing.
I also was able to remove the casting of the spawned EnElf to EnItem00 in the non-random drop functions: actors are now responsible for casting if they are doing something with the spawned item. This seems better since the actor is the one controlling the type of actor spawned, so it ought to know what type it should be.
I have also not attempted to document any of the other params used in EnItem00, mainly because their interaction is a bit of a mystery to me and gets tied up into the actual actor, and I wanted to get a PR done without significant scope creep for once.
Apparently this is one of the cases where != 0 gives different codegen... probably a point in favour of no macro for 0.
The point is that previously it was basically impossible to associate the dropped item and the number dropped. Originally I made the ROW macros to group them, but then I realised that even with it was very difficult to read them. This seems to me to be a much clearer way to do it (even if we wanted to keep it in the same file, which I don't think is a particularly good idea).
(The fact that no one noticed the drop quantities table had extra items at the end rather proves my point.)
It seems like this data can move down to the Item_ section too, is that something we'd like to do?
I think it's enough to highlight each drop table (each "row"), to have comments between each:
static u8 sItemDropIds[] = {
// 0
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_BLUE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_GREEN,
ITEM00_MAGIC_SMALL,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_MAGIC_SMALL,
ITEM00_FLEXIBLE,
ITEM00_SEEDS,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_RUPEE_BLUE,
// 1
ITEM00_RUPEE_GREEN,
ITEM00_MAGIC_SMALL,
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_BLUE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_FLEXIBLE,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_MAGIC_SMALL,
// 2
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_GREEN,
ITEM00_MAGIC_SMALL,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_SEEDS,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_FLEXIBLE,
ITEM00_MAGIC_SMALL,
// 3
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_GREEN,
ITEM00_NUTS,
ITEM00_NONE,
ITEM00_SEEDS,
ITEM00_SEEDS,
ITEM00_NUTS,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_FLEXIBLE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
// 4
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_GREEN,
ITEM00_SEEDS,
ITEM00_BOMBS_A,
ITEM00_MAGIC_SMALL,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_MAGIC_SMALL,
// 5
ITEM00_RUPEE_GREEN,
ITEM00_MAGIC_SMALL,
ITEM00_RUPEE_GREEN,
ITEM00_NONE,
ITEM00_RUPEE_BLUE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_FLEXIBLE,
ITEM00_SEEDS,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_MAGIC_SMALL,
// 6
ITEM00_RUPEE_GREEN,
ITEM00_RUPEE_BLUE,
ITEM00_NONE,
ITEM00_RUPEE_GREEN,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_ARROWS_SMALL,
ITEM00_NONE,
ITEM00_ARROWS_MEDIUM,
ITEM00_MAGIC_SMALL,
ITEM00_FLEXIBLE,
ITEM00_NONE,
ITEM00_MAGIC_LARGE,
// 7
ITEM00_RUPEE_GREEN,
ITEM00_NONE,
ITEM00_RUPEE_BLUE,
ITEM00_NONE,
ITEM00_RUPEE_GREEN,
ITEM00_HEART,
ITEM00_FLEXIBLE,
ITEM00_BOMBS_A,
ITEM00_ARROWS_SMALL,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_MAGIC_SMALL,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_MAGIC_LARGE,
// 8
ITEM00_ARROWS_LARGE,
ITEM00_ARROWS_MEDIUM,
ITEM00_ARROWS_MEDIUM,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_SMALL,
ITEM00_FLEXIBLE,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_MEDIUM,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_SMALL,
ITEM00_ARROWS_MEDIUM,
ITEM00_ARROWS_LARGE,
ITEM00_ARROWS_LARGE,
// 9
ITEM00_MAGIC_LARGE,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_LARGE,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_LARGE,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_LARGE,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_SMALL,
ITEM00_MAGIC_LARGE,
// 10
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_FLEXIBLE,
ITEM00_BOMBS_A,
ITEM00_BOMBS_A,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_BOMBS_A,
ITEM00_NONE,
ITEM00_BOMBS_A,
// 11
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_HEART,
// 12
ITEM00_RUPEE_RED,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_BLUE,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_RED,
ITEM00_RUPEE_RED,
// 13
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_NUTS,
ITEM00_NONE,
ITEM00_STICK,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_NUTS,
ITEM00_NONE,
ITEM00_NUTS,
ITEM00_HEART,
ITEM00_SEEDS,
// 14
ITEM00_HEART,
ITEM00_NONE,
ITEM00_SEEDS,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_NONE,
ITEM00_HEART,
ITEM00_SEEDS,
ITEM00_FLEXIBLE,
};
static u8 sDropQuantities[] = {
// 0
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 1
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 2
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 3
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
0,
0,
0,
0,
// 4
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
3,
1,
1,
1,
1,
// 5
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
3,
1,
1,
1,
1,
1,
// 6
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 7
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 8
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 9
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 10
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
0,
1,
// 11
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
3,
// 12
3,
3,
3,
1,
3,
3,
3,
1,
1,
3,
1,
3,
1,
1,
1,
3,
// 13
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
// 14
3,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
1,
3,
1,
1,
};
Sure it is less convenient than having the data merged but it doesn't look worth it in this case, as it's just these two arrays.
Also this data isn't a major thing, editing it directly looks ok, table-ifying minor things seems like Pandora's box.
I can think of other candidates for such table-ification: src/code/z_debug.c's inputCombos and regChar, player's sActionModelGroups, D_80853EDC and D_80853FE8, basically any set of array indexed by the same thing
Yes, I think it would be worth doing for most if not all of them, and anything else of a substantial size where you'd have to have things in multiple places line up for shifting purposes; this is actually one of the less useful such places. I don't really see why we wouldn't do this, really, your example takes out so much vertical space that in some ways it's even less readable than what we currently have.
Yeah my example takes twice the proposed include/tables/drop_tables.h in vertical space, but I feel like not relying on a new file, and (imo needlessly) convoluted macros, is worth it (or rather, the new file and macros are not worth two times less lines)
Due to inactivity from the author, this PR along with all others currently open from Elliptic will be closed 1 week from today. All of these PRs have been open for more than a year with no clear intentions of upkeep from the author.
This message will not be copy pasted on the other PRs, but they also apply to: https://github.com/zeldaret/oot/pull/1346 https://github.com/zeldaret/oot/pull/1374 https://github.com/zeldaret/oot/pull/1330
Elliptic, if you intend to get back into maintaining these PRs and engaging in discussions in the near future, let me know before September 13th that you wish for these to remain open.
Thank you.
