Tip: When determining a time limit, remember that players won't be as familiar with the level as you are. If you normally complete the level with around 100s left on the timer, others might run out of time on their first try.
In levels with layer 3 tides, the properties of the current normally apply to all swimmable areas, including bodies of water above the current. This patch restricts the pushing to the tide region, allowing water above it to be swimmable as normal.
Much of the claim time for this was scrounging up enough time to write this log, I apologize for that.
Anyhow, this patch is being rejected for not doing what it's supposed to do in all reasonably encountered scenarios. Specifically, moving up high on the screen can cause the player to start getting pushed around by the tide, despite being nowhere near the layer 3 water:
Small as this patch already is, there are actually quite a few ways it can be further streamlined. I want to stress that efficiency is not remotely part of the rejection reason here - this patch just happens to be an excellent case study for some intermediate-level optimization techniques, and I'd just hate to squander a teachable moment. None of what follows is intended to be a critique of your work or abilities, so I hope none of it comes across as presumptuous!
The original code, for reference:
This code can be trimmed down as below. Points of interest are marked and detailed further down:
;y position where the tide starts taking effect. it is relative to the tide's position on screen.
;I had to tune it myself so I left it here as a define, but I don't think you need to mess with it.
!Offset = $00E8
!long = $800000
if read1($00FFD5) == $23
!long = $000000
autoclean JML WalkSpeed ; (1) ;\this seems to control player walking speed
NOP ; ;/when under a tide
JML AccLeft ; (2) ;this seems to be about the auto speed given to the player
freedata ; (3)
BEQ Return ;
TAY ; (4)
JSR TideCheck ;
TYA ; (5)
BCC Return ; (6)
ADC #$03 ; (7)
JML $00DA5D|!long ; (8)
JSR TideCheck ;
BCC NoAccLeft ;
LDX #$1E ;
LDA $72 ;
JML $00DA75|!long ;
JML $00DA79|!long ;
REP #$21 ; (9)
LDA $80 ; (10)
ADC $24 ;
CMP #!Offset ;
SEP #$20 ;
Your "seems" implies a lack of certainty, but I can confirm your assessment of this routine is correct. It handles Mario's acceleration and max speed when underwater specifically when holding Left or Right on the D-pad, with the hijack point in particular deciding between "tide speeds" and "normal speeds".
Your assessment is more or less correct here, as well. This code handles Mario's deceleration and automatic movement when given neither Left nor Right D-pad input. It uses the same framework for automatic movement as is used for auto-sliding down steep slopes.
Your routine doesn't change the data bank, and may thus use freedata instead of freecode.
The WalkSpeed routine is entered with a base index to water movement speeds in A. The objective of this routine is to increase A by four if Mario is below the tide level (switch it to "tide movement speeds"), and leave it alone if above (leave it as "normal water movement speeds"). The subroutine that determines whether Mario is below the tide level destroys A, so in the original code A is pushed and then pulled. Because there's a branch in the middle, each path requires its own PLA in order to ensure stack integrity.
Upon inspection, however, this routine still returns to $00DA5D no matter which branch is taken. If we look at the code there, the first thing we see is TAY - in other words, Y is immediately overwritten. This tells us that we're free to use Y as we please in our routine, since its current value won't be used later. The in-progress swim speed index is thus preserved in Y rather than the stack, saving cycles.
The TideCheck subroutine sets the carry flag if Mario is below the tide level. As far as processor flags go, carry is particularly robust as many opcodes don't touch it, allowing you to "delay" acting on a check while you prep other data. We stuck the original speed index in Y; we need to exit the routine with some form of this index in A no matter what, so we move it back to A here. This action does not affect carry as was just set by the TideCheck subroutine.
Now that the index is back in A, the carry flag decides whether we are below the tide (and thus need to add four) or above it (and can simply jump straight to a return).
If we make it to this location in the code, we have the advantage of knowing that carry is always set, no matter what. ADC adds its argument, plus an additional #$01 if the carry flag is set. Since it's always set here, ADC #$03 will always add #$04 as we require, allowing us to discard a redundant CLC opcode.
There's nothing at all wrong or suboptimal about using labels to set up your return addresses, I just wrote the returns in the style I most prefer. It does, however, make for a good excuse to bring up and encourage the practice of including FastROM mirrors.
The REP opcode resets processor flags of choice. This is typically done to clear the accumulator/register size flags to enable 16-bit A/X/Y. Here, we change REP #$20 to REP #$21, which will clear the accumulator size flag to enable 16-bit A but will also clear the carry flag as well.
The LDA opcode never alters the carry flag, which per the above we have made to always be clear at this point. And because carry is guaranteed to be clear here, we may remove a redundant CLC opcode.
This address here - $80 - is the reason that tide detection gets confused when Mario is high up on the screen. This address is meant to be the player Y position within the borders of the screen. It's calculated as the difference between 16-bit $96 (player Y position in the level) and $1C (layer 1 Y position)*. The problem is, it's possible for $96 to be less than $1C; this causes $80 to go negative, which your tide level detection subroutine doesn't expect. The remedy should be as simple as immediately branching to RTS if $80 is negative (which, if using the REP #$21 trick, would elegantly set the correct water speed index for this case).
* Plus or minus some additional inconsequential fluff; see $00E346 for the full calculation.
Tested with Asar 1.81, Lunar Magic 3.21, SA-1 1.32, Snes9x 1.59.2.