openai / retro

Retro Games in Gym
MIT License
3.36k stars 525 forks source link

Exception in update_ram() #115

Open christopherhesse opened 5 years ago

christopherhesse commented 5 years ago

Issue summary

import retro

games = [
    'StarFox-Snes',
    'JikkyouOshaberiParodius-Snes',
    'StarFox-Snes',
]

for game in games:
    print(game)
    env = retro.make(game=game)
    env.reset()
    env.step(env.action_space.sample())
    env.close()

Running this script on my machine will often work but sometimes segfault or print this exception:

StarFox-Snes
Sound buffer size: 128160 (32040 samples)
JikkyouOshaberiParodius-Snes
Sound buffer size: 128160 (32040 samples)
StarFox-Snes
Sound buffer size: 128160 (32040 samples)
Traceback (most recent call last):
  File "/Users/csh/agi/sandbox/oneoff/retro-state-fail.py", line 12, in <module>
    env.reset()
  File "/Users/csh/agi/retro/retro/retro_env.py", line 227, in reset
    self.data.update_ram()
IndexError: No known mapping
python ~/agi/sandbox/oneoff/retro-state-fail.py
StarFox-Snes
Sound buffer size: 128160 (32040 samples)
JikkyouOshaberiParodius-Snes
Sound buffer size: 128160 (32040 samples)
[1]    69484 segmentation fault  python ~/agi/sandbox/oneoff/retro-state-fail.py

System information

christopherhesse commented 5 years ago

The StarFox-Snes issues were caused by two bugs in the snes emulator:

diff --git a/libretro/libretro.cpp b/libretro/libretro.cpp
index f66920f..7898521 100644
--- a/libretro/libretro.cpp
+++ b/libretro/libretro.cpp
@@ -481,7 +481,7 @@ static bool merge_mapping()
    }
    uint32 len=a->len;
    if (!len) len=(0x1000000 - a->select);
-   if (len && ((len-1) & (len | a->disconnect))==0 && ((char*)a->ptr)+a->offset+len == ((char*)b->ptr)+b->offset)
+   if (len && ((len-1) & (len | a->disconnect))==0 && a->start + len == b->start && ((char*)a->ptr)+a->offset+len == ((char*)b->ptr)+b->offset)
    {
 //printf("merge/consec\n");
        a->select &=~ len;
diff --git a/memmap.cpp b/memmap.cpp
index edb8b64..ae238bb 100644
--- a/memmap.cpp
+++ b/memmap.cpp
@@ -3228,10 +3228,10 @@ void CMemory::Map_SufamiTurboPseudoLoROMMap (void)
    map_lorom_offset(0xc0, 0xdf, 0x8000, 0xffff, 0x100000, 0x200000);

    // I don't care :P
-   map_space(0x60, 0x63, 0x8000, 0xffff, SRAM - 0x8000);
-   map_space(0xe0, 0xe3, 0x8000, 0xffff, SRAM - 0x8000);
-   map_space(0x70, 0x73, 0x8000, 0xffff, SRAM + 0x4000 - 0x8000, false);//these two seem to duplicate the above ones in
-   map_space(0xf0, 0xf3, 0x8000, 0xffff, SRAM + 0x4000 - 0x8000, false);//way that makes the duplicates hard to find.
+   map_space(0x60, 0x63, 0x8000, 0xffff, SRAM);
+   map_space(0xe0, 0xe3, 0x8000, 0xffff, SRAM);
+   map_space(0x70, 0x73, 0x8000, 0xffff, SRAM + 0x4000, false);//these two seem to duplicate the above ones in
+   map_space(0xf0, 0xf3, 0x8000, 0xffff, SRAM + 0x4000, false);//way that makes the duplicates hard to find.

    map_WRAM();

@@ -3257,8 +3257,8 @@ void CMemory::Map_SuperFXLoROMMap (void)
    map_hirom_offset(0x40, 0x7f, 0x0000, 0xffff, CalculatedSize, 0);
    map_hirom_offset(0xc0, 0xff, 0x0000, 0xffff, CalculatedSize, 0);

-   map_space(0x00, 0x3f, 0x6000, 0x7fff, SRAM - 0x6000);
-   map_space(0x80, 0xbf, 0x6000, 0x7fff, SRAM - 0x6000);
+   map_space(0x00, 0x3f, 0x6000, 0x7fff, SRAM);
+   map_space(0x80, 0xbf, 0x6000, 0x7fff, SRAM);
    map_space(0x70, 0x70, 0x0000, 0xffff, SRAM);
    map_space(0x71, 0x71, 0x0000, 0xffff, SRAM + 0x10000);

The JikkyouOshaberiParodius-Snes issue could be also a memory map issue, but I didn't look at it. It's not clear to me if the map_space() fix is correct.

christopherhesse commented 5 years ago

The memory map stuff was removed upstream: https://github.com/libretro/snes9x/commit/29bfa104bdfd51c47ad6c41af438ba6c2c623390#diff-0ea06cc6ec68b10cd243d508e9996308

tylerroost commented 5 years ago

Currently running into this problem with pokemon blue. I believe my data.json file to be accurate, and I can get it to run when it the scenario file makes no calls to the data.json file ie its this: { } Wondering how to progress forward

My problem is happening from the call to self.data.update.ram() is happening from retro_env.py

christopherhesse commented 5 years ago

That sounds more like you don't have a valid memory address in data.json

tylerroost commented 5 years ago

My numbers are in the same format as other gameboy data.json files in the retro build. So i don't think I do and they show up in the integration UI but again are at max value due to a bug in the integration UI

christopherhesse commented 5 years ago

Did you try the solution here? https://github.com/openai/retro/issues/147

tylerroost commented 5 years ago

Working on implementing now

ljcarlin commented 3 years ago

I'm encountering the same error trying to do a custom integration of the original SimCity on SNES. I built my data.json file using the integration tool to find the addresses of variables.

Issue summary

import retro
import os

SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))

retro.data.Integrations.add_custom_path(os.path.join(SCRIPT_DIR, "custom_integrations"))
env = retro.make("SimCity-Snes", inttype=retro.data.Integrations.ALL)
env.reset()

When I run the above I get the following error

Traceback (most recent call last):
    File "testrun.py", line 8,, in <module>
        env.reset()
    File "home/louis/wm/gym-retro/retro/retro_env.py", line 206, in reset
        self.data.update_ram()
IndexError: No known mapping

I also get the same issue running other methods of the GameData class which interact with game memory, like data.set_value() for example.

data.json:

{
  "info": {
      "buildtype": {
      "address": 8258061,
      "type": "|u1"
    },
    "money": {
      "address": 8260509,
      "type": "<i4"
    },
    "population": {
      "address": 8260517,
      "type": "<u2"
    }
  }
}

scenario.json:

{
 "done": {
    "variables": {
      "population": {
        "op": "greater-than",
        "reference": 1000
      }
    }
  },
  "reward": {
    "variables": {
      "population": {
        "reward": 1.0
      }
    }
  }
}

System Info 0.8.0.dev5+g6886936 Python 3.8.6 Arch linux

christopherhesse commented 3 years ago

I don't see anything obviously wrong with your setup, if you try with different variables removed, can you narrow it down to one particular variable you are defining?

ljcarlin commented 3 years ago

Thanks for the quick response!

The issue seems to occur with any variable (I tried a few others as well). As I mentioned it can also be replicated by calling any of the methods which interact with game memory. An empty scenario.json avoids the error, but only because it means that env.reset() does not involve game memory.

I tried the included Airstriker-Genesis rom and I'm able to access the memory with no issues. I'll try replicate it on a supported SNES rom once I work out where to locate the actual rom. I wonder if it could be a SNES specific issue potentially?

christopherhesse commented 3 years ago

Very possible

ljcarlin commented 3 years ago

Running BattletoadsDoubleDragon-Snes I'm able to access game memory without any issues.

I thought it could be an issue with sourcing the rom so I grabbed the USA SimCity rom from the No-Intro Collection which is linked in the documentation. With this I had the same issues.

data.json:

{
  "info": {
    "buildtype": {
      "address": 525,
      "type": "|u1"
    },
    "money": {
      "address": 2973,
      "type": "<i4"
    },
    "population": {
      "address": 2981,
      "type": "<u4"
    }
  }
}

scenario.json:

{
  "done": {
    "variables": {
      "population": {
        "op": "greater-than",
        "reference": 1000
      }
    }
  },
  "reward": {
    "variables": {
      "population": {
        "reward": 1.0
      }
    }
  }
}