fixed mario pipe bug by fixing overflow in add_offset_to_pc(), fixed pushed address in BRK instruction, and cleaned up.

This commit is contained in:
Theron Spiegl 2020-01-08 19:04:01 -06:00
parent 066e977687
commit 4202bbc3b6
9 changed files with 75 additions and 20824 deletions

View File

@ -4,14 +4,10 @@ This is an NES emulator and a work in progress. The CPU and PPU work, though the
- One dependency (SDL)
- One line of `unsafe` (`std::mem::transmute::<u8>() -> i8`)
- NTSC timing
<img src="pics/smb.png" width=600>
<sup>(Warning: this pipe currently takes you to an empty room, it's not the only one, and I don't know why.)</sup>
## Controls:
```
Button | Key

16352
SMBDIS.ASM

File diff suppressed because it is too large Load Diff

3076
dump

File diff suppressed because it is too large Load Diff

View File

@ -1,6 +1,6 @@
extern crate sdl2;
use sdl2::audio::{AudioCallback, AudioSpecDesired};
use sdl2::audio::AudioSpecDesired;
pub fn initialize(context: &sdl2::Sdl) -> Result<sdl2::audio::AudioQueue<f32>, String> {
let audio_subsystem = context.audio()?;

View File

@ -83,8 +83,6 @@ pub struct Cpu {
pub strobe: u8,
pub button_states: u8, // Player 1 controller
button_number: u8,
more: usize,
}
impl Cpu {
@ -104,7 +102,6 @@ impl Cpu {
strobe: 0,
button_states: 0,
button_number: 0,
more: 0,
opcode_table: vec![
// 00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
/*00*/ Cpu::brk, Cpu::ora, Cpu::bad, Cpu::slo, Cpu::nop, Cpu::ora, Cpu::asl, Cpu::slo, Cpu::php, Cpu::ora, Cpu::asl, Cpu::nop, Cpu::nop, Cpu::ora, Cpu::asl, Cpu::slo, /*00*/
@ -172,46 +169,9 @@ impl Cpu {
// get addressing mode
let mode = self.mode_table[opcode].clone();
let (address_func, num_bytes) = mode.get();
let (address_func, _num_bytes) = mode.get();
let address = address_func(self);
// debugging
// assert!(self.memory_at(0xAD79, 141) == UNDERGROUND_LEVEL.to_vec() && self.memory_at(0xA133, 45) == UNDERGROUND_ENEMIES.to_vec());
// let pc = self.PC;
// if address == 0x06D6 {
// // let mem = self.memory_at(0xAD79, 141);
// // println!("memory at 0xAD79: {:02X?}", mem);
// println!("===========================\n0x{:04X} {:?}", address, mode);
// if self.more == 0 {
// self.more += 24;
// }
// }
// if pc == 0xB1E5 {
// println!("===========================");
// if self.more == 0 {
// self.more += 24;
// }
// }
// if self.more > 0 {
// let operands = match num_bytes {
// 1 => " ".to_string(),
// 2 => format!("{:02X} ", self.read(pc + 1)),
// 3 => format!("{:02X} {:02X}", self.read(pc + 1), self.read(pc+2)),
// _ => "error".to_string(),
// };
// print!("{:04X} {:02X} {} {} A:{:02X} X:{:02X} Y:{:02X} P:{:02X} SP:{:02X}",
// pc, self.read(pc), operands, OPCODE_DISPLAY_NAMES[opcode],
// self.A, self.X, self.Y, self.P, self.S,
// );
// // let mut zpg = Vec::<u8>::new();
// // for i in 0..32 {
// // zpg.push(self.read(i));
// // }
// // print!(" zpg: {:x?}", zpg);
// print!("\n");
// self.more -= 1;
// }
// self._debug(_num_bytes, opcode);
// advance program counter according to how many bytes that instruction operated on
self.advance_pc(mode);
// look up instruction in table and execute
@ -221,14 +181,6 @@ impl Cpu {
self.clock - clock
}
pub fn memory_at(&mut self, address: usize, amount: usize) -> Vec<u8> {
let mut ret = vec![];
for i in 0..amount {
ret.push(self.read(address+i));
}
ret
}
// memory interface
pub fn read(&mut self, address: usize) -> u8 {
let val = match address {
@ -249,23 +201,6 @@ impl Cpu {
// memory interface
fn write(&mut self, address: usize, val: u8) {
// if address == 0x06D6 {
// println!("writing 0x{:02X} to 0x{:04X}", val, address);
// }
let vars = vec![
("PlayerEntranceCtrl", 0x0710),
("AltEntranceControl", 0x0752),
("EntrancePage", 0x0751),
("AreaPointer", 0x0750),
("AreaAddrsLOffset", 0x074f),
];
for i in vars.iter() {
if i.1 == address {
println!("writing 0x{:02X} to {}", val, i.0);
}
}
match address {
0x0000..=0x1FFF => self.mem[address % 0x0800] = val,
0x2000..=0x3FFF => self.write_ppu_reg(address % 8, val),
@ -336,8 +271,28 @@ impl Cpu {
}
}
}
fn _debug(&mut self, num_bytes: usize, opcode: usize) {
let pc = self.PC;
let operands = match num_bytes {
1 => " ".to_string(),
2 => format!("{:02X} ", self.read(pc + 1)),
3 => format!("{:02X} {:02X}", self.read(pc + 1), self.read(pc+2)),
_ => "error".to_string(),
};
println!("{:04X} {:02X} {} {} A:{:02X} X:{:02X} Y:{:02X} P:{:02X} SP:{:02X}",
pc, self.read(pc), operands, _OPCODE_DISPLAY_NAMES[opcode],
self.A, self.X, self.Y, self.P, self.S,
);
}
pub fn _memory_at(&mut self, address: usize, amount: usize) -> Vec<u8> {
let mut ret = vec![];
for i in 0..amount {
ret.push(self.read(address+i));
}
ret
}
}
/*
Address range Size Device
@ -353,7 +308,7 @@ $4020-$FFFF $BFE0 Cartridge space: PRG ROM, PRG RAM, and mapper registers (See
*/
// For debug output
const OPCODE_DISPLAY_NAMES: [&str; 256] = [
const _OPCODE_DISPLAY_NAMES: [&str; 256] = [
"BRK", "ORA", "BAD", "SLO", "NOP", "ORA", "ASL", "SLO",
"PHP", "ORA", "ASL", "ANC", "NOP", "ORA", "ASL", "SLO",
"BPL", "ORA", "BAD", "SLO", "NOP", "ORA", "ASL", "SLO",
@ -387,21 +342,3 @@ const OPCODE_DISPLAY_NAMES: [&str; 256] = [
"BEQ", "SBC", "BAD", "ISC", "NOP", "SBC", "INC", "ISC",
"SED", "SBC", "NOP", "ISC", "NOP", "SBC", "INC", "ISC",
];
// const UNDERGROUND_LEVEL: [u8; 141] = [
// 0x48, 0x01, 0x0e, 0x01, 0x00, 0x5a, 0x3e, 0x06, 0x45, 0x46, 0x47, 0x46, 0x53, 0x44, 0xae, 0x01,
// 0xdf, 0x4a, 0x4d, 0xc7, 0x0e, 0x81, 0x00, 0x5a, 0x2e, 0x04, 0x37, 0x28, 0x3a, 0x48, 0x46, 0x47,
// 0xc7, 0x07, 0xce, 0x0f, 0xdf, 0x4a, 0x4d, 0xc7, 0x0e, 0x81, 0x00, 0x5a, 0x33, 0x53, 0x43, 0x51,
// 0x46, 0x40, 0x47, 0x50, 0x53, 0x04, 0x55, 0x40, 0x56, 0x50, 0x62, 0x43, 0x64, 0x40, 0x65, 0x50,
// 0x71, 0x41, 0x73, 0x51, 0x83, 0x51, 0x94, 0x40, 0x95, 0x50, 0xa3, 0x50, 0xa5, 0x40, 0xa6, 0x50,
// 0xb3, 0x51, 0xb6, 0x40, 0xb7, 0x50, 0xc3, 0x53, 0xdf, 0x4a, 0x4d, 0xc7, 0x0e, 0x81, 0x00, 0x5a,
// 0x2e, 0x02, 0x36, 0x47, 0x37, 0x52, 0x3a, 0x49, 0x47, 0x25, 0xa7, 0x52, 0xd7, 0x04, 0xdf, 0x4a,
// 0x4d, 0xc7, 0x0e, 0x81, 0x00, 0x5a, 0x3e, 0x02, 0x44, 0x51, 0x53, 0x44, 0x54, 0x44, 0x55, 0x24,
// 0xa1, 0x54, 0xae, 0x01, 0xb4, 0x21, 0xdf, 0x4a, 0xe5, 0x07, 0x4d, 0xc7, 0xfd,
// ];
// const UNDERGROUND_ENEMIES: [u8; 45] = [
// 0x1e, 0xa5, 0x0a, 0x2e, 0x28, 0x27, 0x2e, 0x33, 0xc7, 0x0f, 0x03, 0x1e, 0x40, 0x07, 0x2e, 0x30,
// 0xe7, 0x0f, 0x05, 0x1e, 0x24, 0x44, 0x0f, 0x07, 0x1e, 0x22, 0x6a, 0x2e, 0x23, 0xab, 0x0f, 0x09,
// 0x1e, 0x41, 0x68, 0x1e, 0x2a, 0x8a, 0x2e, 0x23, 0xa2, 0x2e, 0x32, 0xea, 0xff,
// ];

View File

@ -119,8 +119,19 @@ impl super::Cpu {
}
pub fn brk(&mut self, _address: usize, _mode: Mode) {
self.push((self.PC >> 8) as u8); // push high byte
self.push((self.PC & 0xFF) as u8); // push low byte
// instr_test-v5/rom_singles/15-brk.nes and instr_test-v5/rom_singles/16-special.nes:
// using self.PC + 1 in these next two lines allows these tests to pass.
// I'm not sure why that's necessary as implied addressing mode is only supposed to consume 1 byte,
// but the error message from 16-special.nes said "BRK should push address BRK + 2"
// Aha! From http://nesdev.com/the%20%27B%27%20flag%20&%20BRK%20instruction.txt:
// Regardless of what ANY 6502 documentation says, BRK is a 2 byte opcode. The
// first is #$00, and the second is a padding byte. This explains why interrupt
// routines called by BRK always return 2 bytes after the actual BRK opcode,
// and not just 1.
self.push(((self.PC + 1) >> 8) as u8); // push high byte
self.push(((self.PC + 1) & 0xFF) as u8); // push low byte
self.push(self.P | 0b00110000); // push status register with break bits set
self.P |= INTERRUPT_DISABLE_FLAG; // set interrupt disable flag
self.PC = ((self.read(IRQ_VECTOR + 1) as usize) << 8) // set program counter to IRQ/BRK vector, taking high byte
@ -326,24 +337,25 @@ impl super::Cpu {
pub fn plp(&mut self, _address: usize, _mode: Mode) {
self.clock += 2;
let status = self.pop();
// for each bit in the popped status, if it's 1,
// set that bit of self.P to 1. if it's 0, set that
// bit of self.P to 0.
for i in 0..=7 {
if i == 4 || i == 5 {
continue; // ignore B flags
}
let bit = if status & (1 << i) == 0 {0} else {1};
if bit != 0 {
self.P |= 1 << i;
} else {
self.P &= 0xFF - (1 << i);
}
}
self.P |= 1 << 5; // turn on bit 5
self.P &= 0xFF - (1 << 4); // and turn off bit 4 because god knows why
self.P = self.pop();
// TODO: figure out exactly what's supposed to happen here
// let status = self.pop();
// // for each bit in the popped status, if it's 1,
// // set that bit of self.P to 1. if it's 0, set that
// // bit of self.P to 0.
// for i in 0..=7 {
// if i == 4 || i == 5 {
// continue; // ignore B flags
// }
// let bit = if status & (1 << i) == 0 {0} else {1};
// if bit != 0 {
// self.P |= 1 << i;
// } else {
// self.P &= 0xFF - (1 << i);
// }
// }
// self.P |= 1 << 5; // turn on bit 5
// self.P &= 0xFF - (1 << 4); // and turn off bit 4 because god knows why
}
pub fn rla(&mut self, _address: usize, _mode: Mode) {

View File

@ -27,8 +27,10 @@ impl super::Cpu {
self.PC += decoded_offset;
},
false => {
let decoded_offset = (-offset) as usize;
self.PC -= decoded_offset;
// instr_test-v5/rom_singles/11-stack.nes:
// letting decoded_offset be (-offset) as usize was allowing for overflow if offset was -128/0b10000000
let decoded_offset = (-offset) as u8;
self.PC -= decoded_offset as usize;
},
}
}
@ -46,7 +48,7 @@ impl super::Cpu {
}
pub fn branch(&mut self, unsigned_offset: u8) {
let offset: i8 = u8_to_i8(unsigned_offset);
let offset = unsigned_offset as i8;
self.clock += 1;
let old_addr = self.PC;
self.add_offset_to_pc(offset);
@ -93,7 +95,3 @@ impl super::Cpu {
}
}
pub fn u8_to_i8(offset: u8) -> i8 {
unsafe { std::mem::transmute(offset) }
}

View File

@ -50,28 +50,6 @@ fn main() -> Result<(), String> {
let mut fps = 0;
let mut sps = 0;
// TODO: remove
// check for location of VerticalPipeEntry
// println!("verticalPipeEntry: {:02X?}", cpu.memory_at(0xB225, 512));
// why not just dump all memory?
// let mut mem = cpu.memory_at(0, 0x4020);
// let mut mem2 = cpu.memory_at(0x8000, 0xFFFF-0x8000);
// mem.append(&mut mem2);
// let mut line = 0;
// for i in 0..0x4020 {
// if i % 0x10 == 0 {
// print!("\n0x{:04X}: ", i);
// }
// print!("{:02X} ", mem[i]);
// }
// println!("\n=========================");
// for i in 0x8000..=0xFFFF {
// if i % 0x10 == 0 {
// print!("\n0x{:04X}: ", i);
// }
// print!("{:02X} ", mem[i-0x4020]);
// }
// PROFILER.lock().unwrap().start("./main.profile").unwrap();
'running: loop {
// step CPU: perform 1 cpu instruction, getting back number of clock cycles it took
@ -130,11 +108,11 @@ fn main() -> Result<(), String> {
// calculate fps
let now = Instant::now();
if now > fps_timer + Duration::from_secs(1) {
// println!("fps: {}", fps);
println!("fps: {}", fps);
fps = 0;
fps_timer = now;
// println!("samples per second: {}", sps);
println!("samples per second: {}", sps);
sps = 0;
}
@ -144,12 +122,12 @@ fn main() -> Result<(), String> {
}
/*
TODO:
- common mappers
- DMC audio channel, high- and low-pass filters, refactor envelope
- name audio variables (dividers, counters, etc.) more consistently
- common mappers
- battery-backed RAM solution
- fix mysterious Mario pipe non-locations
- GUI? drag and drop ROMs?
- reset function
- save/load/pause functionality
@ -161,56 +139,14 @@ The SDL audio device samples/outputs at 44,100Hz, so as long as the APU queues u
But it's not doing so evenly. If PPU runs faster than 60Hz, audio will get skipped, and if slower, audio will pop/have gaps.
Need to probably lock everything to the APU but worried about checking time that often. Can do for some division of 44_100.
Nowhere room debugging:
Do we want to detect every time WarpZoneControl is accessed and log a buffer before and after it?
Or is the problem not with loading WZC but writing it? Good and bad logs match when entering the pipe.
The subroutine that accesses $06D6 is HandlePipeEntry. That's only called by ChkFootMTile->DoFootCheck->ChkCollSize->PlayerBGCollision->PlayerCtrlRoutine.
PlayerCtrlRoutine is called by PlayerInjuryBlink and PlayerDeath, and all three of those are called by GameRoutines engine.
So the normal physics loop checks for pipe entry every so often. So need to find out how HandlePipeEntry determines where to send you,
and what puts you in the room.
Failed tests from instr_test-v5/rom_singles/:
3, immediate, Failed. Just unofficial instructions?
0B AAC #n
2B AAC #n
4B ASR #n
6B ARR #n
AB ATX #n
CB AXS #n
7, abs_xy, 'illegal opcode using abs x: 9c'
Functions that write to WarpZoneControl:
WarpZoneObject<-RunEnemyObjectsCore<-EnemiesAndLoopsCore<-VictoryMode/GameEngine
ScrollLockObject_Warp<-DecodeAreaData<-ProcessAreaData<-
Is ParseRow0e a clue?
I think L_UndergroundArea3 is the data for the coin rooms. Need to verify that it's loaded properly.
It's at 0x2D89 in the ROM, so 0x2D79 without header. Which means it's in PRG ROM, because it's within the first 0x4000,
in the first PRG chunk/vec given to CPU by cartridge. Because it's NROM, that will be mapped starting at $8000,
so its position in memory should be 0x8000 + 0x2D79 = 0xAD79.
L_UndergroundArea3 is indeed at 0xAD79 and correct in both good emulator and mine. So need to detect its use? Verified that
it's not changed, neither is E_UndergroundArea3 which is at $A133. WarpZoneControl is also set properly: 0 for a while, then
1 when running over exit in 2-1 to Warp Zone, then 4 once dropped down into the WarpZone. 0 when going into any coin rooms.
HandlePipeEntry queues VerticalPipeEntry:
sta GameEngineSubroutine ;set to run vertical pipe entry routine on next frame
Then it checks WarpZoneControl and branches to rts if :
lda WarpZoneControl ;check warp zone control
beq ExPipeE ;branch to leave if none found
[...]
ExPipeE: rts ;leave!!!
So the problem may be in VerticalPipeEntry. Need to hook it. It starts with lda #$01, so looking for lda in immediate mode, which is 0xA9
followed by jsr then followed by a two byte absolute address we don't know, so 0x20 ?? ??, then jsr another function, so same thing,
then ldy #$00, which is 0xA0 0x00... so now we can grep the rom file for its address and compare to good emulator.
grep -A10 "a9 *01 *20 *.. *.. *20 *.. *.. *a0"
000031f0 52 07 4c 13 b2 a9 01 20 00 b2 20 93 af a0 00 ad |R.L.... .. .....|
VerticalPipeEntry is at $31F5 in the ROM, so at $B205 in the running emulator. Now need to confirm that and then log starting there.
No, had to do a full memory dump to find out that it's at $B225... Anyway, can now hook there. But hook was wrong. And hooking for address == $06D6
shows the program counter at 0xB1EF, meaning I was right that the routine's address is 0xB1E5... So my dump was wrong? Or routines move around? Doesn't make sense.
Anyway, hook PC == $B1E5.
Ok, so, comparing logs with the good emulator down the WORKING pipe in 1-1 shows a divergence in behavior based on loading value 0x6E from $0755 into the accumulator,
and comparing that to 0x50. What's at $0755? Player_Pos_ForScroll, which is just Mario's horizontal position on screen.
And that's the only difference, over and over, so doesn't really matter.
Now need to see what's different when we drop into the bad room. 1200 lines in log from one pipe,
25 lines each (24 and a separator) so 48 iterations of VerticalPipeEntry per pipe.
But logs for VerticalPipeEntry for the bad room also seem to match the good emulator except for the Player_Pos_ForScroll...
Other suspicious variables:
PlayerEntranceCtrl
AltEntranceControl
EntrancePage
AreaPointer
AreaAddrsLOffset
*/

File diff suppressed because it is too large Load Diff