| File Name: | Super Mario Bros. Wonder - Music Blocks |
|---|---|
| Submitted: | by Nyako |
| Authors: | Nyako |
| Act As: | 130 |
| Includes GFX: | No |
| Description: | Those silly little noteblock variants from Super Mario Bros. Wonder! You can walk or run on them, and when you jump, it bounces you up! The block has a few configuration options you'll need to set in the block, being the sprite it spawns when you step on it (defaults to custom sprite 0), the page you're inserting the blocks into, and the IDs of the blocks you've inserted into, to get the colors correctly. The sprite also has configuration options for the palettes it uses, the sprite tile, and how high it launches the player. Further usage instructions are in the README. |
| Tags: | lorom, note block, sa-1, smbw, sprite |
| Screenshots: |  |
Major:
- The primary reason for the rejection is that the block doesn't correctly check to make sure that the helper sprite spawned before erasing itself and attempting to set it up. If there are no available sprite slots, then at best, the block will simply vanish, and at worst, it's writing to unintended areas of memory, which could result in strange behavior, and possibly even a crash under the right circumstances. It looks like a check was attempted, but it's way after the call to spawn the sprite, which could even cause other issues.
- The Y register needs to be preserved by the block. This contains the high byte of the act-as for the block, and can change its behavior to something unintentional if it's not preserved.
Medium:
- There's really no need to check map16 page in the block code -- it wouldn't be running if it were something else anyway
- Likewise, to get the needed index, it would be easier (for both the code and the end user) to simply specify the map16 value of the first block; then you can just subtract that from the current block's map16 value. This would require having all the blocks be consecutive in map16, but that's fairly reasonable to do. This would also allow someone to easily use a pattern with more or fewer colors, too. For exmample, there could be a define at the beginning such as
!FirstBlockMap16 = $0320 or whatever- All the entries in the Palettes: table in the sprite has the bits set for priority 3 (making the ORA $64 pointless) and gfx page 1 hardcoded. There are multiple ways to handle this, but you should still either respect the priority value in $64 (and the tweaker bit setting for second gfx page), or let the user specify their own.
- These don't work correctly when placed on layer 2 (the sprites will usually spawn at the wrong Y position at least, and then they'll put the blocks back onto layer 1). You don't have to make them work on layer 2, but you should at least have a note that this is the case
Minor:
- You might want to consider having the main block code run for TopCorner: in addition to MarioAbove:, since I was able to stand on the corner without having the block get triggered
- You get a big bounce from the block by dismounting Yoshi while on one. This may or may not be intentional. It's certainly fine if you want to keep it this way, but I wanted to point it out in case it wasn't intended.
- Sprites (including Yoshi) will fall through activated blocks, which can be kind of odd. I think the donut lift blocks have the same issue, so it's not a critical fix or anything, but it might be worth considering replacing with an invisible sprite-solid block instead to prevent this.
- It's certainly not necessary, but it might be nice to include a graphic for this with the submission (like in the screenshot), or at least pointing to an existing one if that's what you used
- While I'm normally a strong supporter of using .json files for sprites instead of .cfgs, this is actually one case where a .cfg might be more appropriate. That's the case here because these sprites aren't meant to be placed in LM, so they just wind up taking space in the list of custom sprites, and won't work right if they're placed anyway.