kodi-game / kodi-game-scripting

Scripting for Kodi Game add-ons
GNU General Public License v2.0
5 stars 9 forks source link

ctypes getting core variables/settings does not always work #69

Open zach-morris opened 4 years ago

zach-morris commented 4 years ago

Full disclosure, I'm trying to troubleshoot my own code utilizing kodi-game-scripting, but I think this would likely effect kodi-game-scripting's ability to generate the official addons as well.

The issue I've found with this code is that it doesn't consistently pull core settings from the binary. Here's an example:

from lib.kodi_game_scripting.libretro_ctypes import LibretroWrapper
library_data = LibretroWrapper('ecwolf_libretro.dylib')
library_data.variables
Out[4]: []

library_data = LibretroWrapper('4do_libretro.dylib')
library_data.variables
Out[8]: 
[Variable(id='4do_bios', description='BIOS (rom1)', values=['disabled'], default='disabled'),
 Variable(id='4do_font', description='Font (rom2)', values=['disabled'], default='disabled'),
 Variable(id='4do_cpu_overclock', description='CPU Overclock', values=['1.0x (12.50Mhz)', '1.1x (13.75Mhz)', '1.2x (15.00Mhz)', '1.5x (18.75Mhz)', '1.6x (20.00Mhz)', '1.8x (22.50Mhz)', '2.0x (25.00Mhz)'], default='1.0x (12.50Mhz)'),
 Variable(id='4do_region', description='Mode', values=['ntsc', 'pal1', 'pal2'], default='ntsc'),
 Variable(id='4do_vdlp_pixel_format', description='VDLP Pixel Format', values=['XRGB8888', '0RGB1555', 'RGB565'], default='XRGB8888'),
 Variable(id='4do_vdlp_bypass_clut', description='VDLP Bypass CLUT', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_high_resolution', description='HiRes CEL Rendering', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_madam_matrix_engine', description='MADAM Matrix Engine', values=['hardware', 'software'], default='hardware'),
 Variable(id='4do_swi_hle', description='OperaOS SWI HLE', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_nvram_storage', description='NVRAM Storage', values=['per game', 'shared'], default='per game'),
 Variable(id='4do_active_devices', description='Active Input Devices', values=['1', '0', '2', '3', '4', '5', '6', '7', '8'], default='1'),
 Variable(id='4do_hack_timing_1', description="Timing Hack 1 (Crash 'n Burn)", values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_hack_timing_3', description='Timing Hack 3 (Dinopark Tycoon)', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_hack_timing_5', description='Timing Hack 5 (Microcosm)', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_hack_timing_6', description='Timing Hack 6 (Alone in the Dark)', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_hack_graphics_step_y', description='Graphics Step Y Hack (Samurai Showdown)', values=['disabled', 'enabled'], default='disabled'),
 Variable(id='4do_kprint', description='Debug Output', values=['disabled', 'enabled'], default='disabled')]

From Retroarch directly, I can see the first binary should have the following variables:

ecwolf-adlib-volume = "20"
ecwolf-alwaysrun = "enabled"
ecwolf-am-drawtexturedfloors = "enabled"
ecwolf-am-drawtexturedwalls = "enabled"
ecwolf-am-overlay = "on"
ecwolf-am-pause = "enabled"
ecwolf-am-rotate = "off"
ecwolf-am-showratios = "enabled"
ecwolf-am-texturedoverlay = "enabled"
ecwolf-analog-move-sensitivity = "20"
ecwolf-analog-turn-sensitivity = "20"
ecwolf-aspect = "auto"
ecwolf-digi-volume = "20"
ecwolf-dynamic-fps = "disabled"
ecwolf-effects-priority = "digi-adlib-speaker"
ecwolf-fps = "60"
ecwolf-invulnerability = "disabled"
ecwolf-music-volume = "20"
ecwolf-resolution = "1280x800"
ecwolf-speaker-volume = "20"
ecwolf-viewsize = "20"

The problem seems to be here, where the correct RETRO_ENVIRONMENT_SET_VARIABLES cb_type is never called to pull the variables for the first binary. Based on the code, I can't figure out why however.

garbear commented 4 years ago

I noticed this too, and resulted in not updating several add-ons where the settings "disappeared". My thoughts were it's probably related to the new options api: https://github.com/libretro/RetroArch/blob/master/libretro-common/include/libretro.h#L1109

Basically we need to set the int pointed to by arg to 1. As python doesn't do pointers, I have no clue how to do this. E.g. in C++ it would be:

*arg = 1;
garbear commented 4 years ago

@Rechi are you familiar with python ctypes?

zach-morris commented 4 years ago

I'll note now that I'm looking that some addons have settings that are somewhat garbled, showing multiple instances of the same possible setting values now in many addons. game.libretro.uae for example:

Screen Shot 2020-03-02 at 6 59 51 PM Screen Shot 2020-03-02 at 6 59 45 PM

settings.xml shows:

<setting label="30001" type="select" id="puae_analogmouse" values="right|disabled|left|right|both" default="right"/>
        <setting label="30002" type="select" id="puae_analogmouse_deadzone" values="15|0|5|10|15|20|25|30|35" default="15"/>
        <setting label="30003" type="select" id="puae_analogmouse_speed" values="1.0|0.5|0.6|0.7|0.8|0.9|1.0|1.1|1.2|1.3|1.4|1.5" default="1.0"/>
...
zach-morris commented 4 years ago

Playing around with this, I got halfway there. A new cb_type=53 needs to be added, which I think was an API change for libretro.

Something like:

            elif cb_type==53:  # RETRO_ENVIRONMENT_SET_CORE_OPTIONS
                index = 0
                while True:
                    var = ctypes.cast(arg, ctypes.POINTER(
                        self.RetroOptions))[index]
...

But the part I can't figure out is the definition of the RetroOptions. The 'values' are structs within structs and I dont understand ctypes fully:

    class RetroOptions(ctypes.Structure):
        """ struct libretro_options """
        _fields_ = [
            ('key', ctypes.c_char_p),
            ('desc', ctypes.c_char_p),
            ('info', ctypes.c_char_p),
            ('value', ???)
        ]
garbear commented 4 years ago

Good progress so far. I have a feeling a nontrivial understanding of ctypes is needed. Per the API, cb_type=53 should only be called if cb_type=52 sets an int at the specified address pointed to by the argument, and pointer indirection isn't something I know how to with ctypes today

garbear commented 3 years ago

The API function we need to handle is RETRO_ENVIRONMENT_GET_CORE_OPTIONS_VERSION: https://github.com/libretro/RetroArch/blob/master/libretro-common/include/libretro.h#L1117. The parameter is unsigned *, which means it looks like this:

void environment_cb(RETRO_ENVIRONMENT_GET_CORE_OPTIONS_VERSION, void *data)
{
  unsigned int *version = reinterpret_cast<unsigned int*>(data);
  *version = 2;
}

So in ctypes, you take the void* parameters from the function, and interpret it as an unsigned int *, then assign the number 2 to the memory pointed to by the pointer.

zach-morris commented 1 year ago

Just adding some tinkering I've done with this.

In libretro_ctypes.py I created two additional structs:

class retro_core_option_value(ctypes.Structure):
    _fields_ = [("value", ctypes.c_char_p),
                ("label", ctypes.c_char_p)]

class retro_core_option_definition(ctypes.Structure):
    _fields_ = [("key", ctypes.c_char_p),
                ("description", ctypes.c_char_p),
                ("info", ctypes.c_char_p),
                ("values", ctypes.POINTER(retro_core_option_value)*RETRO_NUM_CORE_OPTION_VALUES_MAX),
                ("default_value", ctypes.c_char_p),
                ]

And then created another cb_type for testing:

elif cb_type == 53:  #RETRO_ENVIRONMENT_SET_CORE_OPTIONS
                num_options = 0
                while True:
                    core_option = ctypes.cast(arg, ctypes.POINTER(retro_core_option_definition))[num_options]
                    num_options+=1
                    if core_option.key is None and core_option.values is None:
                        break
                    else:
                        num_values = 0
                        if core_option.key is not None:
                            print("Core option key:", core_option.key or '')
                        if core_option.description is not None:
                            print("Core option short description:", core_option.description or '')
                        if core_option.info is not None:
                                print("Core option info:", core_option.info or '')
                        if core_option.default_value is not None:
                            print("Core option default_value:", core_option.default_value or '')
                        while True:
                            core_option_value = ctypes.cast(core_option.values,ctypes.POINTER(retro_core_option_value))[num_values]
                            num_values+=1
                            if core_option_value.value is None and core_option_value.label is None:
                                break
                            else:
                                if core_option_value.value is not None:
                                    print("Core options value:", core_option_value.value or '')
                                if core_option_value.label is not None:
                                    print("Core options label:", core_option_value.label or '')

This method does pull some of the settings for RETRO_ENVIRONMENT_SET_CORE_OPTIONS, but it quickly gets out of sync. I believe the issue is because retro_core_option_value struct is dynamically sized vs. actually sized to the MAX. My best stackexchange searching can't get past how to dynamically define the struct size within the cb call (I don't speak C, so thats probably the issue, and maybe thats what you were referring to with assigning the number 2 to the memory pointed to by the pointer.).

garbear commented 1 year ago

Unfortunately I followed pretty much the exact same path and hit a wall when I couldn't find the documentation I needed to do certain things. I think the solution is to create a lightweight C++ program that loads the core, calls the retro_* functions, and translates their results to stdout, where it's then parsed by the Python code.

garbear commented 1 year ago

Actually, I just remembered I already did this: https://github.com/garbear/repository.libretro/tree/master/libretro-extract. We could use the libretro-extract utility to get settings from the libretro options v2 API.