search

bugzmanov / nes_ebook

A mini book on writing NES emulator using rust lang
https://bugzmanov.github.io/nes_ebook/index.html
385 stars 68 forks source link

Bug in get_operand_address -> Indirect_X ? #8

Closed bschlenk closed 3 years ago

bschlenk commented 3 years ago

Hi, first of all thank you for this tutorial! I've really been struggling trying to wrap my head around writing an NES emulator, and this has been a great resource.

I'm thinking this might be a bug or just me misunderstanding something about rust, but this line looks like it is doing a wrapping_add of the u8 pointer, which is not how the emulator from https://skilldrick.github.io/easy6502/ is working. Given this code:

LDX #$00
LDA #$05
STA $ff
LDA #$07
STA $0100
LDA #$08
STA $00
LDY #$0d
STY $0705
LDY #$0b
STY $0805
LDA ($ff,X)

(you can plug it in here)

If the u8 pointer were wrapped, I'd expect it to load A from $0805, but it is loading from $0705, since we end up with A having a value of $0b.

I think this code should instead be:

let lo = self.mem_read(ptr as u16);
let hi = self.mem_read((ptr as u16).wrapping_add(1));
bschlenk commented 3 years ago

I ended up asking about this on stack overflow, seems like what you have is correct, and the emulator is wrong. Either that, or I'm not following this assembly code right.

Feel free to close this.

bugzmanov commented 3 years ago

@bschlenk Hey Brian. Thank you for taking time to write this up!

I believe your conclusion is correct. The lookup table space for Indirect_X is limited by 8 bits. That's why it should be (ptr as u8).wrapping_add(1) for the hi part.

Honestly, I had struggled quite a bit with Indirect operations. Eventually I got it right because of the test rom that covers a lot of corner cases for CPU instructions (https://bugzmanov.github.io/nes_ebook/chapter_5_1.html)

Hope this helps. Cheers!