Proposed enhancement to Uzebox Kernel

[Artcfox]

So I ran into a bug with my WIP that could easily be fixed with a tiny modification to the Mode 3 kernel.

The Background:

There is an entity spawning system, so enemies that haven't been seen yet (or have strayed too far off screen) don't take up any RAM or CPU. When they get spawned into existence, some of them use GetVsyncCounter() in order to know the global time so things like sprite animations can remain in sync with the sprite animation of an entity right next to it that may have been spawned into existence at a slightly different time. More importantly, the major rotation of the firebars is tied to this counter value, so if you walk offscreen and the firebar despawns, when you get near that section of the level again it spawns back in with the same "position on the clock" as if it had never been despawned at all.

The Problem:

Press START to bring up the pause menu while a firebar is onscreen, and then press START again to close the pause menu. It will immediately jump to a different clock position as if the game had never been paused. Because for it, and anything in the game that uses GetVsyncCounter() to keep track of world time, the world time didn't stop when the game was paused.

The Solution:

Code: Select all

      // Begin to display pause menu
      uint16_t backupVsyncCounter = GetVsyncCounter();
...
      // (pause menu stuff)
...
      // About to close pause menu

      // Restore the backed up VsyncCounter so time doesn't advance for the entities while the game is paused
      // (For this to compile, you need to add ".global vsync_counter" to the top of uzeboxVideoEngineCore.s)
      extern uint16_t vsync_counter;
      vsync_counter = backupVsyncCounter;

Currently there is a GetVsyncCounter(), and a ClearVsyncCounter(), but no SetVsyncCounter() in the kernel. I wouldn't want to add that and bloat the kernel for all every other video mode, because that might break the games out there that use up all of the available space, so my proposal is to just expose the vsync_counter variable the same way that some of the other variables are exposed in uzeboxVideoEngineCore.s like the following:

Code: Select all

;Public variables
.global sync_pulse
.global sync_phase
.global sync_flags
.global joypad1_status_lo
.global joypad2_status_lo
.global joypad1_status_hi
.global joypad2_status_hi
.global first_render_line
.global render_lines_count
.global vsync_counter

This variable is not used anywhere else by the kernel and the comments indicate that it can be cleared and read by user code at any time to use for timing purposes. Does anyone have any objections to me making that a public variable so it can also be set or restored to a specific counter value?

[Artcfox]

After talking with CunningFellow, putting it in its own function section will not cause it to be included if it's not being used, so making it a proper function won't bloat the kernel like I was worried about.

Attached is my proposed patch. Unzip, and apply it with the command:

Code: Select all

git apply setvsynccounter.patch

in the uzebox/ directory.

For quick viewing/review, here are the contents of the zipped .patch file, but in order for the diff to apply cleanly, you will need to use the zipped patch file with the above git apply command, since those files still have DOS line endings, and any patches also need to have DOS line endings:

Code: Select all

diff --git a/kernel/uzebox.h b/kernel/uzebox.h
index 0458275..f06effa 100644
--- a/kernel/uzebox.h
+++ b/kernel/uzebox.h
@@ -82,6 +82,7 @@
 	extern   u8 GetVsyncFlag(void);
 	extern void ClearVsyncCounter();
 	extern u16	GetVsyncCounter();	
+        extern void SetVsyncCounter(u16 count);
 
 	extern void SetRenderingParameters(u8 firstScanlineToRender, u8 verticalTilesToRender);
 
diff --git a/kernel/uzeboxVideoEngineCore.s b/kernel/uzeboxVideoEngineCore.s
index 7b6e36f..f801aef 100644
--- a/kernel/uzeboxVideoEngineCore.s
+++ b/kernel/uzeboxVideoEngineCore.s
@@ -90,6 +90,7 @@
 .global IsRunningInEmulator
 .global GetVsyncCounter
 .global ClearVsyncCounter
+.global SetVsyncCounter
 
 ;Public variables
 .global sync_pulse
@@ -553,6 +554,18 @@ ClearVsyncCounter:
 	sts vsync_counter+1,r1
 	ret
 
+;************************************
+; Set the vsync counter.
+;
+; C-callable
+; r25:r24=(unsigned int)count
+;************************************
+.section .text.SetVsyncCounter
+SetVsyncCounter:
+        sts vsync_counter,r24
+        sts vsync_counter+1,r25
+        ret
+
 
 ;*****************************
 ; Return joypad 1 or 2 buttons status

[Artcfox]

This is now in master along with a compiler warning fix.

I also added API documentation for SetVsyncCounter, and the missing API documentation for the pre-existing ClearVsyncCounter and GetVsyncCounter functions to the wiki.

If anyone knows when those API functions had been added, the version number might need to be corrected. I also wasn't sure what version number we are on now, so I just put today's date for when SetVsyncCounter was added.