Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heuristically find Level_BG_Pages constant arrays #143

Closed
fortenbt opened this issue Nov 14, 2022 · 12 comments · Fixed by #144
Closed

Heuristically find Level_BG_Pages constant arrays #143

fortenbt opened this issue Nov 14, 2022 · 12 comments · Fixed by #144

Comments

@fortenbt
Copy link
Contributor

The two constant arrays, Level_BG_Pages1 and Level_BG_Pages2 are used in foundry/game/gfs/GraphicsSet.py to find the graphics banks for each tileset.

For the most part, these bank arrays are not going to be modified in a hack, but could easily change their location within prg030.asm if the ROM hacker modified prg030.asm.

Ideally, some emulation could be used within the WorldMap_Loop function to detect which location is used as the Level_BG_Pages constants, but you get into a recursive problem needing to know where the WorldMap_Loop or level loading code begins. At some point, you're using byte search values or constant locations anyway.

So, for a first pass, just finding the stock array bytes could easily work for many hacks that modify prg030.asm and the editor could display the graphics correctly.

@mchlnix
Copy link
Owner

mchlnix commented Nov 14, 2022

To deal with the issue of changing label locations, I thought that users could supply a .fns file (I think), which lists all named labels and their absolute address in the ROM. That's how I got these values in the first place, by using such a file from the stock ROM. You can get this from the assembler, so anyone doing real ASM work, should be able to produce such a file. Code for that should already be in the constants.py file somewhere. But the constants wouldn't be updated, I think.

@fortenbt
Copy link
Contributor Author

I would try to make sure any assembler-related solution took into account nesasm and asm6f. Some users do use Drakim's asm6 port, and Mesen has very good debugger integration for asm6f.

@mchlnix
Copy link
Owner

mchlnix commented Nov 14, 2022

Do they produce the same fns type file, or are there different versions?

@fortenbt
Copy link
Contributor Author

They don't. The fns file format is pretty limited, and doesn't contain enough information to know which bank the symbol came from, so you don't know absolute ROM location without also assuming the bank number. ASM6f corrects this with better symbol file support (Mesen .mlb files and FCEUX *.nl files).

NESASM doesn't have a good output format for symbols that gives you everything you need.

@fortenbt
Copy link
Contributor Author

fortenbt commented Nov 14, 2022

Honestly, after looking around, I would only support asm6f-produced files. Based on what I've seen of the debugger, Mesen is by far the best there is, and its Mesen label format (.mlb) looks like its the most encompassing format.

It sucks to give up nesasm, but I think you gain a lot more from Mesen and FCEUX's name labels.

@mchlnix
Copy link
Owner

mchlnix commented Nov 14, 2022

They don't. The fns file format is pretty limited, and doesn't contain enough information to know which bank the symbol came from, so you don't know absolute ROM location without also assuming the bank number.

I'm not quite sure what you mean, can you not assume that bank numbers are in order? So $address // 0x2000 = PRG number?

@fortenbt
Copy link
Contributor Author

The fns files contain linked RAM addresses, not ROM addresses. So prg030's and prg031's symbols' addresses are all unique between $8000-$9FFFF and $E000-$FFFF, but all the other banks assembled at $A000-$BFFF and $C000-$DFFF have overlapping addresses.

@mchlnix
Copy link
Owner

mchlnix commented Nov 14, 2022

I had a look and you are right about the fns. In my assorted SMB3 folder, next to the fns file I also found a mlb file. Not sure if I parsed that to get the constants I have in the .py file. I can remember talking to BlueFinch about the nasm so maybe it has an option with more useful addresses.

But at least there is an option so we can work off of that.

@fortenbt
Copy link
Contributor Author

I was thinking through a design for bringing in MLB (Mesen Label File) labels produced by ASM6f.

The way the hierarchy is currently set up, we have smb3parse.constants that has a bunch of constants. I'm proposing changing smb3parse.constants into a package that defines an MlbLabel and MlbFile. The Settings Dialog could have a section for "Disassembly," where a path could be added to the disassembly and an MLB label file could be given.

Eventually, a lot of stuff could be done with the disassembly path, and for this issue, we could use the MLB file (if defined) to replace all the constants usage. I've added an example implementation of MlbLabel and MlbFile below.

from functools import partial
import sys 

class MlbLabel:
    GetAddr = partial(int, base=16)

    def __init__(self, line: str) -> None:
        self._fields = line.split(":")
        self.type, self.addr_str, self._name = self._fields[0:3]
        self.comment = None if len(self._fields) < 4 else self._fields[3]

    @property
    def addr(self) -> int:
        return MlbLabel.GetAddr(self.addr_str)

    @property
    def name(self) -> str:
        return self._name.strip()


class MlbFile:
    def __init__(self, fname: str) -> None:
        self.labels = []
        with open(fname, 'r') as f:
            for line in f:
                self.labels.append(MlbLabel(line))

The smb3parse.constants package could provide get_symbol_rom_addr, get_symbol_ram_addr, etc. If the MLB file isn't given in settings yet, we bring along an MLB file produced from the vanilla disassembly and initialize with that.

@mchlnix
Copy link
Owner

mchlnix commented Nov 16, 2022

Hm. I'm thinking about how the MLB file should be specified. It's a rom specific thing. And atm there is a settings dialog for the editor as a whole and for the level/world depending on the editor. Which brings up the next thing, this would need to be done for Scribe as well.

This is stuff an IDE would normally put into a Project file. Now we could do things like remember the path of a rom and, if we encounter it again, restore its settings, including the MLB file, but that seems to be too much magic for something that gets copied and backed up and renamed as often as I imagine a ROM would.

Then comes the question of what happens if you set the MLB for your super ASM heavy hack and you want to open a Vanilla ROM. How would we gracefully fail? Or would the user have to remember and even recognize that maybe only small things are going wrong behind the scenes? Then again, we already have that problem if someone would open an ASM heavy hack, like 3Mix for example with Vanilla offsets.

I think this feature request, of being able to provide new labels for heavily modified hacks is more advanced than the editor itself is at the moment. Watching some of the recent hackathon people are not even going beyond vanilla level bounds or moving Levels manually by setting their address.

Putting focus into making that experience easier should probably take precedence. I honestly don't even know if anyone would need this feature. I assume you would :p

@fortenbt
Copy link
Contributor Author

I can agree with you on most of this. I like the idea of eventually having a project file that both Scribe and Foundry use interchangeably (I would love if Scribe was just a part of Foundry, as I've mentioned to you before).

I guess I would still like this implementation of heuristically finding the correct Level_BG_Pages constants to be merged. It is the most common thing I run into every time I hack away at something to add a feature. These constants are, as far as I've seen, the only things that break using Foundry with a modified prg030 ROM.

If you feel strongly about not merging this 80+% solution (#144) until we have the much larger features involving a project file and disassembly, I get it, and it's easy enough for me to make this change in my local copy when I'm using it.

@mchlnix
Copy link
Owner

mchlnix commented Nov 19, 2022

If it is a problem atm, then it's ok by me ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants