Add platform "MorphOS" #9294

BeWorld2018 · 2024-11-23T08:13:25Z

MorphOS is PPC 32bit bigendian machine (like AmigaOS system)

To build it : make PLATFORM=morphos

Specific to Amiga (MorphOS in my case)

oleg-derevenetz

Hi all,

@BeWorld2018 first of all, thanks for your contribution efforts. Personally, I think that the proposed platform is quite exotic and have significant differences from POSIX in terms of filesystem structure, for instance. I'm not sure if it's worth adding support for such exotic platforms to the main codebase, because there will be issues with supporting this code - MorphOS is a proprietary platform for specific hardware, and I suspect that there are very few people who will be able to, say, test new code or changes in the existing code. The matter is complicated by the fact that, as far as I can see, the proposed changes are not "universal" changes, they are rather "ad hoc" changes.

Alternatively, if MorphOS has its own package manager, I would suggest the author to make and maintain a package (or a separate port) that will contain the necessary patches without integrating them into main codebase.

@ihhub what do you think?

oleg-derevenetz · 2024-11-23T16:22:54Z

src/engine/screen.cpp

+#if defined( _WIN32 ) || defined( __MORPHOS__ )
                if ( fheroes2::cursor().isSoftwareEmulation() ) {
                    flags |= SDL_WINDOW_FULLSCREEN_DESKTOP;
                }
                else {
                    flags |= SDL_WINDOW_FULLSCREEN;
                }
-#else
+


This change is wrong and breaks platforms other than Windows or MorphOS.

Hi @BeWorld2018 , please address this comment.

oleg-derevenetz · 2024-11-23T16:26:01Z

src/engine/screen.cpp

@@ -1090,7 +1090,7 @@ namespace

            uint32_t flags = SDL_WINDOW_SHOWN;
            if ( isFullScreen ) {
-#if defined( _WIN32 )
+#if defined( _WIN32 ) || defined( __MORPHOS__ )


Why use SDL_WINDOW_FULLSCREEN on MorphOS and not SDL_WINDOW_FULLSCREEN_DESKTOP? SDL_WINDOW_FULLSCREEN tends to alter the video card settings and does not work well with multimonitor configurations. On the contrary, we would like to throw the SDL_WINDOW_FULLSCREEN out some day.

SDL_WINDOW_FULLSCREEN on MorphOS use a real new screen, fullscreen desktop use window with no borderless and fullsize...

oleg-derevenetz · 2024-11-23T16:28:18Z

src/engine/serialize.h

@@ -105,6 +116,10 @@

 #include "math_base.h"

+#ifdef __MORPHOS__
+#undef tell


Macros of this kind are always "nice", yeah :)

Why is it done? o_O

Hi @ihhub that's because somewhere inside the system headers of this platform, there is a preprocessor macro with the name tell. Macros have no idea about scopes, namespaces or class methods, which will break the build if your class has its own method with the same name, for instance. Of course, it cannot be said that this is a plus for the platform.

src/engine/system.cpp

oleg-derevenetz · 2024-11-23T16:46:36Z

src/engine/zzlib.cpp

@@ -29,6 +29,10 @@
 #include <zconf.h>
 #include <zlib.h>

+#ifdef __MORPHOS__
+#undef bind


yeah... not nice :-) releated with zlib.h from system headers (shared library)

oleg-derevenetz · 2024-11-23T16:48:23Z

src/fheroes2/game/fheroes2.cpp

+#define STRINGIFY( x ) #x
+#define TOSTRING( x ) STRINGIFY( x )
+#define __AMIGAVER__ TOSTRING( MAJOR_VERSION ) "." TOSTRING( MINOR_VERSION ) "." TOSTRING( INTERMEDIATE_VERSION )
+unsigned long __stack = 1024 * 1024;


Is this some sort of "service variable"?

oleg-derevenetz · 2024-11-23T16:48:29Z

src/fheroes2/game/fheroes2.cpp

+#define TOSTRING( x ) STRINGIFY( x )
+#define __AMIGAVER__ TOSTRING( MAJOR_VERSION ) "." TOSTRING( MINOR_VERSION ) "." TOSTRING( INTERMEDIATE_VERSION )
+unsigned long __stack = 1024 * 1024;
+static const std::string morphos_versions_tag = "$VER: fheroes2 " __AMIGAVER__ " (" __AMIGADATE__ ")";


Where exactly is this variable used and for what?

Specific MorphOS to put version string into executable

oleg-derevenetz · 2024-11-23T16:48:56Z

src/fheroes2/game/fheroes2.cpp

        COUT( GetCaption() )
-
+#endif


What is the issue with this call on MorphOS?

open shell directly with title :-)

@BeWorld2018 , is it true that logging just opens a shell command?

ihhub · 2024-11-24T09:22:12Z

Hi all,

@BeWorld2018 first of all, thanks for your contribution efforts. Personally, I think that the proposed platform is quite exotic and have significant differences from POSIX in terms of filesystem structure, for instance. I'm not sure if it's worth adding support for such exotic platforms to the main codebase, because there will be issues with supporting this code - MorphOS is a proprietary platform for specific hardware, and I suspect that there are very few people who will be able to, say, test new code or changes in the existing code. The matter is complicated by the fact that, as far as I can see, the proposed changes are not "universal" changes, they are rather "ad hoc" changes.

Alternatively, if MorphOS has its own package manager, I would suggest the author to make and maintain a package (or a separate port) that will contain the necessary patches without integrating them into main codebase.

@ihhub what do you think?

Hi @oleg-derevenetz , we are open to expand the range of supported platforms but as you noted it has to be done in a proper way that someone must maintain the code in a proper shape and up to date to avoid any issues and subsequent blame of the core team if a platform fails to support the engine.

@BeWorld2018 , could you please address the given comments. In order to accept the changes for the new platform we should have approved source code changes. Other things are missing in this pull request:

lack of prove that the code is indeed compilable on MorphOS platform. Could you please do the research if any of GitHub actions support this platform? If yes how long it takes to compile fheroes2 using the actions?
we need to expand the documentation to explain how to compile the engine for this platform. Please takes a look at https://github.com/ihhub/fheroes2/blob/master/docs/DEVELOPMENT.md
how to compile SDL 2 on MorphOS? Does it have a package manager which can provide SDL2 package?
is it possible to add fheroes2 as a package for the package manager in MorphOS?

I would recommend to expand the first message in this pull request to include a comprehensive description as well as all steps mentioned by me as tasks, so it would be easier to address them.

I will review this pull request in a while as well. As of now I am marking it as "Draft".

ihhub

Hi @BeWorld2018 , I also put several comment here. Please take a look once you have time.

ihhub · 2024-11-24T09:25:14Z

src/Makefile

@@ -104,7 +107,12 @@ CPPFLAGS := $(CPPFLAGS) $(CPPFLAGS_FH2)
 ifdef FHEROES2_WITH_SYSTEM_SMACKER
 LIBS := $(LIBS) -lsmacker
 endif
+
+ifeq ($(PLATFORM),morphos)
+LIBS := $(LIBS) -lz_shared $(LDLIBS)


Could you please explain why we need to use a shared variant of lz? Can we avoid it? Could you please point to a place where this is the only option available for this platform?

lz_shared use specific shared z library into us SDK. so specific to MorphOS.

ihhub · 2024-11-24T09:25:36Z

src/Makefile.morphos

@@ -0,0 +1,43 @@
+###########################################################################
+#   fheroes2: https://github.com/ihhub/fheroes2                           #
+#   Copyright (C) 2022 - 2024                                             #


Please include only 2024 year.

ihhub · 2024-11-24T09:26:42Z

src/Makefile.morphos

+#   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             #
+###########################################################################
+
+AR = ppc-morphos-gcc-ar-11


Is it a default gcc version for MorphOS? Can we use the default one?

yes it's a last GCC available. but you right we can use ppc-morphos-gcc-ar (default)

Let's use the default one. We want to avoid forcing people to install any other packages just to compile the engine.

ihhub · 2024-11-24T09:27:19Z

src/Makefile.morphos

+LDFLAGS := -L/gg/usr/local/lib $(LDFLAGS) 
+# Common flags (for both C and C++ compilers) for fheroes2 only
+CCFLAGS_FH2 := -DTARGET_MORPHOS
+# -DNDEBUG


Please remove this line as it is not in use anywhere. We have a specific flag for this matter.

ihhub · 2024-11-24T09:28:38Z

src/Makefile.morphos

+LIBS := $(LIBS) -latomic
+
+ifdef FHEROES2_WITH_IMAGE
+LIBS := $(LIBS) -ljpeg -lwebp


Doesn't shell command exist for Makefile on MorphOS? Why we do need to include JPEG and WEBJ libraries here?

worse, i dont build with this flags... FHEROES2_WITH_IMAGE:-) so i dont test this part... need to do.

src/engine/audio.cpp

ihhub · 2024-11-24T09:30:08Z

src/engine/serialize.h

@@ -105,6 +116,10 @@

 #include "math_base.h"

+#ifdef __MORPHOS__
+#undef tell


Why is it done? o_O

oleg-derevenetz · 2024-11-24T10:23:55Z

@ihhub the issue mostly is that fheroes2 expects the target platform to be more or less POSIX-compatible, especially in terms of filesystem structure and file names, and this platform is not POSIX-compatible (at least if it has the same filesystem structure principles as the original Amiga has at the time).

BeWorld2018 · 2024-11-24T12:31:55Z

Hi all,

@BeWorld2018 first of all, thanks for your contribution efforts. Personally, I think that the proposed platform is quite exotic and have significant differences from POSIX in terms of filesystem structure, for instance. I'm not sure if it's worth adding support for such exotic platforms to the main codebase, because there will be issues with supporting this code - MorphOS is a proprietary platform for specific hardware, and I suspect that there are very few people who will be able to, say, test new code or changes in the existing code. The matter is complicated by the fact that, as far as I can see, the proposed changes are not "universal" changes, they are rather "ad hoc" changes.

Alternatively, if MorphOS has its own package manager, I would suggest the author to make and maintain a package (or a separate port) that will contain the necessary patches without integrating them into main codebase.

@ihhub what do you think?

Hi, you certaintly right, MorphOS is little platform :-) and exotic
I can continue to port with my exotic code :-) on my git. no problem for me.

…tems (such as Amiga FS)

oleg-derevenetz · 2024-11-24T21:12:03Z

Hi @BeWorld2018 let's try to implement some improvements step-by-step. I just made a few changes to this PR to implement some "generalization" of the filesystem interaction logic to support non-quite-POSIX-compliant filesystems (which exactly correspond to the changes from #9296). Could you please pull out these changes of mine in your PR and check if everything is working on your device (loading the original game resources, music playback, saving, loading and removing savefiles, map editor, and so on)?

BeWorld2018 · 2024-11-25T11:51:14Z

Hi @BeWorld2018 let's try to implement some improvements step-by-step. I just made a few changes to this PR to implement some "generalization" of the filesystem interaction logic to support non-quite-POSIX-compliant filesystems (which exactly correspond to the changes from #9296). Could you please pull out these changes of mine in your PR and check if everything is working on your device (loading the original game resources, music playback, saving, loading and removing savefiles, map editor, and so on)?

Yeah it's just perfect, just need to exclude MorphOS form GetCaseInsensitibePath, all datas are find with your changes.

Load/Save game = ok
Music = midi ok, ogg ok
Editor : Load/save = ok

oleg-derevenetz · 2024-11-25T12:54:42Z

Hi @BeWorld2018

Yeah it's just perfect, just need to exclude MorphOS form GetCaseInsensitibePath, all datas are find with your changes.

OK, that's good, thank you. The next release will be any day now, after it comes out, we can merge #9296 and #9297 into the master branch to minimize the set of changes specific to MorphOS. After that I'll rebase this PR on top of the master branch, the changeset in this PR of yours will be reduced, and we'll think about what else we can do.

BeWorld2018 · 2024-11-25T14:28:15Z

Nice, thanks you !

oleg-derevenetz · 2024-11-30T06:56:33Z

OK, the file system is more or less sorted out.

Now the question about fullscreen mode - what exactly is the problem with SDL_WINDOW_FULLSCREEN_DESKTOP? Yes, it does not cause video mode switching directly on the graphics card, and instead creates a full-screen borderless window with scaling, but that's exactly what it's supposed to do. This doesn't cause issues with multiple monitors and doesn't cause a mess with other windows or desktop icons due to changing the "real" video mode on the graphics card. What exactly is the problem? Does it somehow not look good on the screen? Does this cause any specific problems when playing? More detailed explanations (perhaps with screenshots) are welcome :)

BeWorld2018 · 2024-12-05T13:37:06Z

OK, the file system is more or less sorted out.

Now the question about fullscreen mode - what exactly is the problem with SDL_WINDOW_FULLSCREEN_DESKTOP? Yes, it does not cause video mode switching directly on the graphics card, and instead creates a full-screen borderless window with scaling, but that's exactly what it's supposed to do. This doesn't cause issues with multiple monitors and doesn't cause a mess with other windows or desktop icons due to changing the "real" video mode on the graphics card. What exactly is the problem? Does it somehow not look good on the screen? Does this cause any specific problems when playing? More detailed explanations (perhaps with screenshots) are welcome :)

No problem here....
On MorphOS this flags SDL_WINDOW_FULLSCREEN_DESKTOP = false fullscreen mean Window take desktop size with borderless (and if you desktop size is 1920x1080... can be slow.. because MorphOS run on G4 / G5 PowerPC... so very old hardware)
and SDL_WINDOW_FULLSCREEN = create new screen with specific size.

oleg-derevenetz · 2024-12-05T15:00:04Z

On MorphOS this flags SDL_WINDOW_FULLSCREEN_DESKTOP = false fullscreen mean Window take desktop size with borderless (and if you desktop size is 1920x1080... can be slow.. because MorphOS run on G4 / G5 PowerPC... so very old hardware)
and SDL_WINDOW_FULLSCREEN = create new screen with specific size.

So the real issue here is that SDL_WINDOW_FULLSCREEN_DESKTOP is slow on high display resolutions? Is this slowdown really being observed?

ihhub · 2024-12-06T00:16:38Z

No problem here.... On MorphOS this flags SDL_WINDOW_FULLSCREEN_DESKTOP = false fullscreen mean Window take desktop size with borderless (and if you desktop size is 1920x1080... can be slow.. because MorphOS run on G4 / G5 PowerPC... so very old hardware) and SDL_WINDOW_FULLSCREEN = create new screen with specific size.

The engine has FPS counter for this matter. Please enable it and see how it affects. Also, do it only on release build.

…o morphos

BeWorld2018 and others added 8 commits February 7, 2024 18:02

MorphOS: support

18e046e

Merge remote-tracking branch 'upstream/master' into morphos

9871d6c

MorphOS: update and cleanup

e3fe8ad

Update

43ad69b

MorphOS: fix -lz with sharedlibs

1359ea7

MorphOS: fix exe version

fa0968e

Specific to Amiga (MorphOS in my case)

Update screen.cpp

b6831cd

Update system.cpp

513f54e

github-actions bot assigned BeWorld2018 Nov 23, 2024

BeWorld2018 added 3 commits November 23, 2024 09:43

Update fheroes2.cpp

aaba6c5

Update system.cpp

bfebbf5

fix

905d238

BeWorld2018 changed the title ~~Add plaform "MorphOS"~~ Add platform "MorphOS" Nov 23, 2024

Update system.cpp

4f47f2c

oleg-derevenetz reviewed Nov 23, 2024

View reviewed changes

ihhub requested changes Nov 24, 2024

View reviewed changes

ihhub marked this pull request as draft November 24, 2024 09:31

oleg-derevenetz added 6 commits November 24, 2024 23:04

Revert the system.cpp changes

e5e05c8

Use the std::filesystem and support non-quite-POSIX-compliant filesys…

59b3a4a

…tems (such as Amiga FS)

Merge branch master into morphos

c29e80c

Update the copyright date

cef4202

Apply the IWYU suggestions

43da355

Make dirSep local

20748b1

Paraphrase the comment

dea5732

oleg-derevenetz mentioned this pull request Nov 25, 2024

Use the platform-dependent byte order for the SDL audio format #9297

Merged

oleg-derevenetz mentioned this pull request Nov 25, 2024

Use the std::filesystem and support non-quite-POSIX-compliant filesystems (such as Amiga FS) #9296

Merged

BeWorld2018 and others added 3 commits November 27, 2024 07:35

Merge remote-tracking branch 'upstream/master' into morphos

344af9d

Merge branch master into morphos

19181eb

Merge branch master into morphos

ca02f8d

Merge branch master into morphos

2b19e5d

oleg-derevenetz and others added 6 commits December 8, 2024 17:43

Merge branch master into morphos

32fa069

Merge branch master into morphos

9a11dbe

Update Makefile.morphos

7ba564f

Merge branch 'morphos' of https://github.com/BeWorld2018/fheroes2 int…

6a14989

…o morphos

Merge remote-tracking branch 'upstream/master' into morphos

dcfca99

Merge remote-tracking branch 'upstream/master' into morphos

8133a8a

Add platform "MorphOS" #9294

Are you sure you want to change the base?

Add platform "MorphOS" #9294

Conversation

BeWorld2018 commented Nov 23, 2024 • edited Loading

oleg-derevenetz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihhub commented Nov 24, 2024 • edited Loading

ihhub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-derevenetz commented Nov 24, 2024

BeWorld2018 commented Nov 24, 2024

oleg-derevenetz commented Nov 24, 2024 • edited Loading

BeWorld2018 commented Nov 25, 2024 • edited Loading

oleg-derevenetz commented Nov 25, 2024 • edited Loading

BeWorld2018 commented Nov 25, 2024

oleg-derevenetz commented Nov 30, 2024 • edited Loading

BeWorld2018 commented Dec 5, 2024

oleg-derevenetz commented Dec 5, 2024

ihhub commented Dec 6, 2024

BeWorld2018 commented Nov 23, 2024 •

edited

Loading

oleg-derevenetz left a comment •

edited

Loading

ihhub commented Nov 24, 2024 •

edited

Loading

oleg-derevenetz commented Nov 24, 2024 •

edited

Loading

BeWorld2018 commented Nov 25, 2024 •

edited

Loading

oleg-derevenetz commented Nov 25, 2024 •

edited

Loading

oleg-derevenetz commented Nov 30, 2024 •

edited

Loading