diablodiab / libretro-scummvm-backend

libretro backend for scummvm
7 stars 8 forks source link

ICB engine is currently broken #17

Closed i30817 closed 1 year ago

i30817 commented 1 year ago

Upstream is including a series of header files that don't appear to get triggered on this build, one of which includes 'stdsdef.h', and one file on upstream, unwisely imo, depends on this:

https://github.com/scummvm/scummvm/pull/3488

The maintainer told me it's not his problem, in spite of best practices saying to import directly what you use, oh well. It's just another patch (to join the current 4):


diff --git a/engines/icb/common/ptr_util.cpp b/engines/icb/common/ptr_util.cpp
index f4ecce3032e..fbbd9df7aba 100644
--- a/engines/icb/common/ptr_util.cpp
+++ b/engines/icb/common/ptr_util.cpp
@@ -27,6 +27,7 @@
 #include "engines/icb/common/px_common.h"
 #include "engines/icb/common/ptr_util.h"

+#include "stddef.h"
 #include "common/array.h"

 namespace ICB {

If you want to, i guess you could also investigate why the scummvm include chain is not including this one. The chain is supposed to be, according to the maintainer:

    engines/icb/common/ptr_util.h at line 30,
    engines/icb/common/px_common.h at line 30,
    common/scummsys.h at line 30,
    /usr/lib/gcc/x86_64-linux-gnu/11/include/stddef.h at line 123

I can't think of a reason why it wouldn't trigger, since all of that code still exists when this repository is put in the scummvm HEAD in the place it should be. Maybe it's hitting the '#if defined(NONSTANDARD_PORT)' on scummsys.h instead of the else.

diablodiab commented 1 year ago

That's seems strange. I'm not having issues compiling the backend as-is on debian and on macos. stddef.h is included in libretro.h

DrUm78 commented 1 year ago

Yes that's weird, this engine compiles fine for me too on Ubuntu 20.04 (WSL2) with GCC 10.2.0. Not sure what version of the backend you have here. 🤔

i30817 commented 1 year ago

I'm on 22.04.1 now on gcc 11.

edit: is libretro.h supposed to be included always even without being imported by the makefile or something?

Or was it that original libretro changed the scummvm.sys file to import it? Wouldn't surprise me. scummvm apparently wants it as a extra file (this is from scummsys.h):


#if defined(NONSTANDARD_PORT)

    // Ports which need to perform #includes and #defines visible in
    // virtually all the source of ScummVM should do so by providing a
    // "portdefs.h" header file (and not by directly modifying this
    // header file).
    #include <portdefs.h>

This file exists in the libretro backend but doesn't import stddef.h

I'm also suspicious that this triggered when i updated the compiler/OS, but it seems stddef.h is on the right place and being picked up when you '#import' it, if you check my previous posts.

i30817 commented 1 year ago

I moved the include to portdefs.h like it seems upstream scummvm intent, that also works:

diff --git a/portdefs.h b/portdefs.h
index dc7707e..5f05070 100755
--- a/portdefs.h
+++ b/portdefs.h
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
 #include <stdarg.h>
 #include <assert.h>

I'd guess if you'd probably want to investigate if libretro.h is actually being used or not.

I still have no explanation of why this triggered when i updated my OS and gcc, so that error (or not) could also be the cause of libretro.h not being included and thus breaking the build of this engine (which appears to be the only one, maybe because it's the only one that doesn't bother to directly import stddef.h directly)

i30817 commented 1 year ago

A few days later...

for some reason, i don't actually understand, portdefs.h also needs now. I'm... pretty sure that upstream didn't change the commandline parser (where this breaks), so i don't even know what's happening with the mysterious not reachable anymore includes.

i30817 commented 1 year ago

Fixed in the new core.