sverx / devkitSMS

development kit and libraries for SEGA Master System / SEGA Game Gear / SEGA SG-1000 / SEGA SC-3000 homebrew programming using C language (and the SDCC compiler)
224 stars 34 forks source link

Add ROM_bank_to_be_mapped_on_slot1/0 symbols #27

Closed raphnet closed 2 years ago

raphnet commented 2 years ago

ROM_bank_to_be_mapped_on_slot2 already exists and can be written to directly to switch bank, and read to know which bank is active.

When using banked code (in slot 1) there are cases where one needs to know which code bank is active, and reading from FFFE is one way to do this.

I'm not sure what the use cases for slot0 are, but defining a symbol to access it should not hurt.

sverx commented 2 years ago

Thought I'm sure no additional symbol should ever hurt, I wouldn't suggest inferring what ROM bank is active from the value you read from any of those RAM locations, as the memory is mirrored and writing at $DFFF for instance would change the value at $FFFF but not the mapper's slot 2 setting. Those addresses are only meant for writing. Not to mention that checking which bank is active would be actually slower than just setting it anyway, even to the same value...

raphnet commented 2 years ago

If a software bug incorrectly write to $DFFF, leading to the mapper setting being out of sync with what memory contains, I would say my project has a very serious issue anyway, and the results of the incorrect active bank assessment are the least of my problem.

In a large project with banked code, I sometimes need to pass the current bank in argument when calling functions in different banks (to pass pointers to static const structures defined inside the current file) and it was quick and easy to get this info from $FFFE. The callee will map slot2 to access the data. I also use this macro to adjust the pointer...

define SLOT1TO2(slot1ptr) ((void*)(((uint16_t)(slot1ptr))+0x4000))

So for instance in file.c compiled with --codeseg BANK7, with a static structure called parameters:

bankedFunctionInOtherBank(ROM_bank_to_be_mapped_on_slot1, SLOT1TO2(&parameters)

I can't argue with the fact that reading ROM_bank_to_be_mapped_on_slot1 is not the fastest method. Now that you mention it I I guess I'll just add -DCODEBANK=7 when compiling the file and use that value directly instead.

Regardless, if it were me I would still add those the library, just in case they are useful or there are other uses cases. (for instance, there could be non-banked code in slot0 meant to be called from any banked function (which would leave slot1 pointing where it was) which could do something automatically based on the current bank mapped to slot 1. The -D approach would not be usable there, as the code in question could be called from any code bank)

But of course, it may open the door to misuse and additional user support. So it's your call and I understand if you do not want this.

sverx commented 2 years ago

It's a very curious use case you have. I wonder if you don't want instead to move the static data the other function needs into the other function's code bank or alternatively in a data bank that you'll map to slot 2 through the usual means. The way I see this is you're trying to use one ROM bank for both banked code and page-able data, and I think this is possible but the two parts should be probably compiled separately with correct instructions for the linker so that the banked code still gets mapped to slot 1 and the static data will be mapped to slot 2 when some function in some other bank needs that. But, apart from saving some space in ROM, this doesn't really give you any advantage, I would say.

As for the slot 0, at the moment there's no plan to support ROM paging there, and I suspect it's something pretty useless, on top of being quite complex to handle.

raphnet commented 2 years ago

In my game I currently have one source file for each enemy type, containing AI code and static information about hit boxes, pointers to tile data (obtained from assets2banks.h), etc.

To me it feels like the static data is at the right place. I mean, if this was a 32 bit system without bank switching, this is where I would like it to be.

I have an enemy manager (handles collisions, movement, etc) in bank 2. AI code for individual enemy types no longer all fit in bank 2. So I cannot just use one bank for enemy stuff (as I was initially doing). I'm adding new enemy AI code to bank 7 now. Each enemy defines in static data structures things such as its hit boxes, pointers to tile data loaded dynamically, etc, and pointers to those structures must be passed to the enemy manager or tile loader.

I also have another problem related to banks and function pointers. Each enemy provides in its structure a "process" function pointer which gets called every frame by the enemy manager. I think sdcc does not support function pointers to banked functions. No matter how I declare it in the structure, when I look at the assembly code it uses the wrong calling convention. So as a workaround, each enemy declares its bank and the enemy manager, knowing it, uses an intermediate function in page 0 to switch to the correct bank before the call, like this:

void monsters_callProcessBanked(struct monster *m) // in page 0
{
    uint8_t savebank;
        // this is called from banked code, slot1 must be restored before returning!
    savebank = ROM_bank_to_be_mapped_on_slot1;

    ROM_bank_to_be_mapped_on_slot1 = m->codebank;
    m->process(m);

    ROM_bank_to_be_mapped_on_slot1 = savebank;
}

So those are my use cases. I initially just added my own definition for ROM_bank_to_be_mapped_on_slot1 in one of my project header files, but I thought it would be worth adding it to SMSlib instead.

I agree that ROM_bank_to_be_mapped_on_slot0 is probably not necessary. Would you like me to update the pull request to only cover slot1? Or would you prefer to also keep ROM_bank_to_be_mapped_on_slot1 out of SMSlib? Let me know and I'll close the pull request in that case.

sverx commented 2 years ago

Yes, I also believe SDCC doesn't support banked function pointers. I never tried it myself but I guess once you save a function address into a function pointer, there's no way for the compiler to retain the _banked attribute and the information about the bank where the code is. Probably a workaround could be to implement something similar to what far pointers are on a PC, but your workaround works fine too.

Regarding this PR, I'm still very unsure about if it's a good idea to add this. I will need some more time to decide.

sverx commented 2 years ago

Let's give more power to the user.

I don't know if anyone will never ever really need slot 0 paging for instance but if someone does need it, he can now do it. :shrug: