Doc/Cleanup pass on object handling

[Dragorn421]

• ObjectStatus status[OBJECT_EXCHANGE_BANK_MAX]; -> ObjectEntry slots[19];
• Object_InitBank -> Object_Init
• Object_Spawn -> Object_SpawnPersistent
• Object_GetIndex -> Object_GetSlot
• "obj bank index" -> "object slot" (objectCtx.slots[actor.objectSlot])
• Cleanup around usage of object slots, and the WaitForObject scheme
• other less important renames and cleanup

Sorry for the big PR, starting to cleanup/rename "obj bank index" stuff was a deeper rabbit hole than I expected it to be. It was far from a replace-all change too, the names in master aren't super consistent

[fig02]

@MNGoldenEagle suggested using the terminology "slot" to help with the "entry" discussion, and I think this could work really well. Ive already called them slots on my own terms when describing things about the system outside of the codebase

ObjectEntry entries[19]; -> ObjectEntry slots[19]; (imo the struct can stay as entry, there are slots for different entries)
Object_GetEntry -> Object_GetSlot
Object_IsEntryLoaded -> Object_IsLoaded (see my pr comment above on why i think this change makes sense)
actor.objectEntry -> actor.objectSlot

unlike with entry, I dont think the word "index" is needed to clarify that its a number for the slot. It works out well imo

[hensldm]

Seems good to me. Hopefully we can agree on a name for func_80097DD8 eventually.

[AngheloAlf]

LGTM

[fig02]

Starting this as a separate comment to start fresh since it has been so long, though technically it is a continuation of #1227 (comment)

I believe that Object_InitContext is a fine name for this. While I do understand concerns that it initializes more than just the literal ObjectContext struct, it gets the point of the function across. We use this method of naming in similar places like Actor_InitContext which does more than just init the literal context structure. It initializes the overlay table, copies the identity matrix to the matrices in play, and inits fw data which mostly concerns save context. It is still actor related things.

In that same vein, I think Object_InitContext is more than suitable for this function name in the same way. Its for the Object system generally.

Note, I named this function with InitContext in the mm version of this PR, which is still under review: zeldaret/mm#1361

[mzxrules]

My current opinion is that Object_InitContext feels like a bad name moreso because the system name of Object_ isn't very good and ideally should be renamed. But if that's not on the cards atm, I agree with fig in calling it Object_InitContext just to get it merged for now.

But if we're up to giving the system a better name, I find it hard not to borrow from og. Perhaps it's because I've been doing a lot of Atari2600 programming lately, but I think I'd prefer to keep the bank swapping terminology given how similar it is to how you'd handle bank switching in older hardware, just in a virtualized form.

For renames I'd personally would like to go with something like this:

Object_ -> ObjectExchange_
ObjectContext -> ObjectExchange
ObjectEntry -> ObjectBank or ObjectExchangeBank
ObjectContext.slot -> ObjectExchange.bankSlot

Object_SpawnPersistent -> ObjectExchange_AllocBank?
func_80097DD8 -> ObjectExchange_Init
Object_UpdateEntries -> ObjectExchange_FetchBanksAsync
Object_GetSlot -> ObjectExchange_GetBankSlot
Object_IsLoaded -> ObjectExchange_IsBankLoaded
func_800981B8 -> ObjectExchange_FetchBanksSync
func_800982FC -> ObjectExchange_BankSwitch

[Dragorn421]

Define "bank"

[mzxrules]

My personal opinion as a programmer is that a bank / memory bank is just a segment of contiguous random access memory. But the definitions I've seen all associate memory banks to physical hardware. For example in bank swapping you aren't spending time transferring data into main memory; a bank swap switches out what physical memory the main processor memory is able to access directly.

In my atari 2600 game for example, I use an 3E+ bankswitching scheme which splits processor memory into 4 1k segments that can be arbitrarily swapped with 64 1k read-only memory segments and 64 0.5k read 0.5k write segments. Bankswitching is done by passing in an 8 bit value storing the main memory segment id and the 0-63 memory bank id to either the read-only memory or read-write register.

More back to Z64, if I were to describe the object swapping system in terms of bank swapping, ObjectEntry would represent the processor accessible memory banks, while the object files themselves would represent external memory banks you'd swap in.

[AngheloAlf]

Can we use a naming convention that better adjusts to this game (or at least to the N64 hardware) instead of terminology from the Atari 2600?
Personally I think the bank terminology doesn't fit the object system

[Dragorn421]

So with the definition of "bank = an area of memory that we can effectively easily swap the data it maps to"
(note: "bank" is not even a good term to use at all from being so poorly defined)

Then the object system doesn't have banks:
You can't swap data in slots besides the highest-indexed/top one, so that fails that definition of "bank"

[hensldm]

Fwiw, Object_InitContext seems fine/best to me.

[mzxrules]

Can we use a naming convention that better adjusts to this game (or at least to the N64 hardware) instead of terminology from the Atari 2600?

Bank switching was still a relevant technique in console game development in the 90s and early 2000s. I only mention the 2600 because I'm more familiar with the system than the Gameboy/Gameboy Color or SNES.

@Dragorn421 Dunno how to best respond, so i'll start off by admitting that I missed that the variable objectCtx->slots[i+1].segment where i is the updated slot happens to be updated when Object_SpawnPersistent and func_800982FC are called (though the later is a bit particular since the update occurs in Scene_CommandObjectList, meaning the system doesn't fully manage it's memory internally).

(note: "bank" is not even a good term to use at all from being so poorly defined)

I don't think so.

So with the definition of "bank = an area of memory that we can effectively easily swap the data it maps to"
Then the object system doesn't have banks: You can't swap data in slots besides the highest-indexed/top one, so that fails that definition of "bank"

Weird thing about that. It's likely intended that slots are reserved sequentially, but func_800982FC's implementation allows any arbitrary slot to be swapped over with a new object file. If you had same sized segments like the warp pad texture objects, you could safely hotswap them with func_800982FC calls, making func_800982FC behave very similarly to swapping banks, with about the same level of safety.

I've thought more on it, and would like to suggest this set of function names instead. I don't think these names are 100% perfect, but at the same time the original system could arguably use a few rewrites to make things cleaner/safer.

Object_SpawnPersistent -> ObjectExchange_AllocPersistentSegment
func_80097DD8 -> ObjectExchange_Init
Object_UpdateEntries -> ObjectExchange_FetchSegmentsAsync
Object_GetSlot -> ObjectExchange_FindSlot <- could stay Get, but Find is a little more accurate imo.
Object_IsLoaded -> ObjectExchange_IsSegmentLoaded
func_800981B8 -> ObjectExchange_FetchSegmentsSync
func_800982FC -> ObjectExchange_AllocSegment

I'm not really a fan of the concept of object files being "spawned".

[mzxrules]

Noticed that the ASSERT here is flawed; It should be checking for cmd->objectList.length + play->objectCtx.numPersistentEntries <= ARRAY_COUNT(play->objectCtx.slots) instead

[fig02]

good catch, though yeah we cant do anything about it cause its their mistake. can add a comment though

[fig02]

@Dragorn421 can add this in this pr if you like, would even consider it an @bug

[Dragorn421]

I would like the names for object_initcontext, object_spawnpersistent, and others, as well as commenting on the flawed assert, to be done in another PR (get this one over with)

[fig02]

Obejct_SpawnPersistent is already named? what else needs to be done there?
I think it makes most sense to name Object_InitContext with this set of changes cause of how important it is. But any other cleanups, I agree

[Dragorn421]

I was referring to mzx' suggestions which seemed to put an emphasis on the spawnpersistent name
I named Object_InitContext