Macros to save/restore the bank mapped to slot2? about devkitsms HOT 9 CLOSED

I prefer the opposite approach: you never assume any data is already mapped into slot 2.
So you map the data when you need to access it. If the calling function needs to re-map some data, it will explicitly do it.
For instance as in:

/*  the program needs to access some data that is in a bank: */
SMS_mapROMBank (some_data_bank);
/*  do some of the processing of the data... */
/* ... then the program needs to load tiles to VRAM - do it: */
SMS_mapROMBank (tiles_bin_bank);
SMS_loadTiles(tiles_bin, 0, tiles_bin_size);
/* re-map the original bank because the program still needs to use it: */
SMS_mapROMBank (some_data_bank);
/* continue as if nothing happened */

from devkitsms.

I like being able to assume that when I set slot2 to the correct bank, it won't unexpectedly change as a result of a function call while I'm doing something. Otherwise it's either difficult to track (you have to know which function may changes it internally - now and in the future) or otherwise you then need a lot of preventive (and in many case unnecessary) SMS_mapROMBank() calls.

Consider this simple example which executes some bytecode in slot2:

runscript() {
  uint8_t *src = scriptbytecode_bin;

  // The script is in a single bank.
  SMS_mapROMBank(scriptbytecode_bin_bank); 

  while(true) {
      // Call the handler for the command, knowing it won't change slot2 (if a handler needs it to load a resource, or access a banked lookup table, it will save and restore it)
      src = handlers_lut[*src](src);
  }
}

Moving the SMS_mapROMBank call inside the while loop above would be no big deal, but in the case of code reading and acting upon fields contained in banked data by calling different functions, it results in a lot of SMS_mapROMBank calls, many possibly unnecessary. It also makes the code longer and slightly harder to read.

from devkitsms.

Though I still don't like that very much I see that in some use case this might be useful so I think we could add that.
Please use unsigned char instead of uint8_t and a nice/longer name for the variable (also I'm not sure why you're declaring the variable in the macro...?)

from devkitsms.

The variable is declared within the macro for convenience to avoid a separate declaration. It's name is prefixed with an underscore to avoid a conflict with user variables within the function where it is used. But I can make it longer and nicer to reduce the chances of conflicts.

Of course, having the declaration in the macro precludes the use of multiple SAVEBANK() statements at the same level in a function. But I think this is fine, since the intention is to have one SAVEBANK() before the first (of potentially multiple) uses of SMS_mapROMBank() in the function, and then one RESTOREBANK() at the end (or before each return). Even though it declares a variable, SAVEBANK() can be used anywhere. (That may not be the case if compiling with -std-c89 however...)

If necessary, SAVEBANK() /RESTOREBANK() pairs may also be nested within blocks, which works well for macros such as SMS_loadTilesBank(). The variable in the "caller", if it exists, is left untouched.

I think the names SMS_saveROMBank() and SMS_restoreROMBank() would be better. What do you think?

from devkitsms.

The names SMS_saveROMBank() and SMS_restoreROMBank() are much better!

I would prefer a global variable, as with local variables you can't issue multiple statements and I think it's not that impossible someone would use this to process some part of data, save, access some other data on a separate bank, restore, then process some more data and again needing to save and access data on another bank again and restore and so on.

So maybe the solution to this one and the #29 is that we need both a global and a local version of those macros? The local versions would be perfect for the interrupt as they wouldn't mess with the global... but it would anyway be up to the programmer to choose which ones to use in which situation of course.

from devkitsms.

Personally I would avoid having a version of the macro with a global variable, since multiple uses in different functions can conflict and lead to unexpectedly restoring a different bank at the end, unless one is extremely careful and attentively tracking which functions may also change the global...

For instance:

function {
  // assume bank currently set to Z
  savebank() // stores bank Z in global variable
  mapRomBank(Y)
  ... do stuff ...
  call a function which also uses a global savebank() // stores Y in global variable
  restoreBank() // points slot2 to bank Y (Wrong! Z was expected)
}

If there were two versions of the macro (global and local) I think I would exclusively use the local version, for interrupt as well as non-interrupt context.

If a user tries to use multiple SAVEBANK() in a same function, they will get a compilation error due to the duplicate declaration and (this is what I would do) investigate the macro definitions. Seeing what they do, I think they should understand how to work around this.

Given the objective of making sure that when the current function returns the bank is at is was (initial value) only one SAVEBANK() should be needed, right at the beginning of the function or at least before the first use of SMS_mapROMBank().

Multiple RESTOREBANK() statements are supported, so if the bank needs to be restored to its initial value meanwhile (for instance, if the function received pointers to data in bank2 in argument), it's entirely possible.

So, say this function is called with slot2 pointing to bank 21:

function(char *dataInCurrentSlot2) {

  // access dataInCurrentSlot2 ...
  // ...

  SAVEBANK(); // saves 21 in local variable
  SMS_mapRomBank(somebank)
  // access data in somebank
  // ... 

  RESTOREBANK(); // slot2 = 21
  // access dataInCurrentSlot2 again
  // ...

  SMS_mapROMBank(someotherbank)
  // access data in someotherbank ...

  // other calls to SMS_mapRomBank(anybank) and RESTOREBANK(), as many as needed

  // final clean up
  RESTOREBANK(); // slot2 = 21
  return;
}

What do you think?

from devkitsms.

You've got a point about the fact that it's not needed to save bank twice to restore it twice.
So likely the local variable version of the macro is the only one anyone would need... hopefully.
OK, let's do it, please use SMS_saveROMBank() and SMS_restoreROMBank() as macro names, thanks :)

from devkitsms.

Created pull request #30

from devkitsms.

Merged! ❤️

from devkitsms.