libsdl-org / SDL-1.2

Simple Directmedia Layer, 1.2 branch ... ***DEPRECATED***, please use https://github.com/libsdl-org/SDL for new projects!
https://libsdl.org
GNU Lesser General Public License v2.1
98 stars 81 forks source link

Set of Atari fixes 2019-2023 #878

Closed mikrosk closed 9 months ago

mikrosk commented 9 months ago

This is my set of fixes and improvements to the atari backend. I had no idea there is still possible to contribute SDL 1.2 so I was keeping them just for myself. Even current Atari maintainer (Patrice Mandin - @pmandin) didn't ask for write access to github. He's convinced me to try to open a PR with my changes, possibly with getting write access as well.

Each commit is compilable and tested by me. I plan to fix a few more things in near future.

sezero commented 9 months ago

I don't like symbols.h residing among public headers. I suggest moving it to a new top-level atari directory and adjusting its users like the following (not tested). I'm willing to merge after such a change.

diff --git a/src/audio/mint/SDL_mintaudio_it.S b/src/audio/mint/SDL_mintaudio_it.S
index 18de7d6..ad97a70 100644
--- a/src/audio/mint/SDL_mintaudio_it.S
+++ b/src/audio/mint/SDL_mintaudio_it.S
@@ -22,3 +22,3 @@

-#include "symbols.h"
+#include "../../../atari/symbols.h"

diff --git a/src/video/ataricommon/SDL_ataric2p.S b/src/video/ataricommon/SDL_ataric2p.S
index 97f3741..33a5136 100644
--- a/src/video/ataricommon/SDL_ataric2p.S
+++ b/src/video/ataricommon/SDL_ataric2p.S
@@ -22,3 +22,3 @@

-#include "symbols.h"
+#include "../../../atari/symbols.h"

diff --git a/src/video/ataricommon/SDL_atarieddi.S b/src/video/ataricommon/SDL_atarieddi.S
index dbc127d..5100e77 100644
--- a/src/video/ataricommon/SDL_atarieddi.S
+++ b/src/video/ataricommon/SDL_atarieddi.S
@@ -22,3 +22,3 @@

-#include "symbols.h"
+#include "../../../atari/symbols.h"

diff --git a/src/video/ataricommon/SDL_ikbdinterrupt.S b/src/video/ataricommon/SDL_ikbdinterrupt.S
index a9ee8c7..bd16d14 100644
--- a/src/video/ataricommon/SDL_ikbdinterrupt.S
+++ b/src/video/ataricommon/SDL_ikbdinterrupt.S
@@ -22,3 +22,3 @@

-#include "symbols.h"
+#include "../../../atari/symbols.h"

diff --git a/src/video/ataricommon/SDL_xbiosinterrupt.S b/src/video/ataricommon/SDL_xbiosinterrupt.S
index d000194..bc64e10 100644
--- a/src/video/ataricommon/SDL_xbiosinterrupt.S
+++ b/src/video/ataricommon/SDL_xbiosinterrupt.S
@@ -22,3 +22,3 @@

-#include "symbols.h"
+#include "../../../atari/symbols.h"
mikrosk commented 9 months ago

I've pushed even less invasive change - moved symbols.h to src/video/ataricommon. I'm aware that the placement isn't exactly great (it's not only video related) but there are other public atari headers placed as well (see e.g. #include "../../video/ataricommon/SDL_atarisuper.h" in src/timer/mint/SDL_systimer.c).

Eventually I'll get to proper separation.

sezero commented 9 months ago

I can't see the patch here?

mikrosk commented 9 months ago

Not sure if it's visible for you too but it shows me "processing updates" in the top right corner... the commit is present in the branch, though: https://github.com/mikrosk/SDL-1.2/commits/to_upstream.

sezero commented 9 months ago

Something is wrong with github I guess, so I pulled your branch to main here, i.e. the patches are in. Thank you!

Closing manually.

icculus commented 9 months ago

Even current Atari maintainer (Patrice Mandin - @pmandin) didn't ask for write access to github.

Patrice, I'm happy to give you write access if you want it. :)