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
atlus/cave.cpp: Added Jumbo Godzilla. #12286
base: master
Are you sure you want to change the base?
Conversation
New Working Software: Jumbo Godzilla [Toho / Namco]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thanks!
I left some notes that may seem or even be a bit pedantic, but trying to keep consistency, when possible, is IMHO a good thing.
Another pet peeve of mine are IMHO superfluous comments like
map(0x100000, 0x10ffff).ram(); // RAM
but I do realize that the whole driver is like that, so I don't think you should remove them.
@@ -21,6 +21,7 @@ Other : 93C46 EEPROM | |||
Year + Game License PCB Tilemaps Sprites Other | |||
----------------------------------------------------------------------------------------- | |||
94 Mazinger Z Banpresto BP943A 038 9335EX706 013 9341E7009 Z80 | |||
95 Jumbo Godzilla Toho N-42 EM 038 9345E7008 013 9442WX002 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the entries in this table are ordered by year, so please put this at least below Power Instinct 2
@@ -71,6 +71,7 @@ class cave_state : public driver_device | |||
void paccarn(machine_config &config); | |||
void paceight(machine_config &config); | |||
void pacslot(machine_config &config); | |||
void jumbogod(machine_config &config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The machine_config methods in this section are in alphabetical order, so please move the jumbogod one accordingly
@@ -247,6 +249,7 @@ class cave_state : public driver_device | |||
void paccarn_map(address_map &map); | |||
void paceight_map(address_map &map); | |||
void pacslot_map(address_map &map); | |||
void jumbogod_map(address_map &map); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods in this section are in alphabetical order, too.
@@ -2204,6 +2204,7 @@ nmaster // (c) 1995 Banpresto / Pandorabox | |||
paccarn // (c) 1996 Namco | |||
paceight // (c) 1996 Namco | |||
pacslot // (c) 1996 Namco | |||
jumbogod // (c) 1995 Namco / Toho |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the lists in mame.lst are supposed to be in alphabetical order (some exception has slipped in, but should be avoided).
Can you please put jumbogod in the right order?
m_led_outputs[0] = data & 0x0001; // tai atari / body slam | ||
m_led_outputs[1] = data & 0x0002; // nissen kougeki / atomic breath | ||
m_led_outputs[2] = data & 0x0004; // shippo kougeki / tail attack | ||
m_led_outputs[3] = data & 0x0008; // nissen kougeki / atomic breath | ||
m_led_outputs[4] = data & 0x0010; // motor & spinning lamps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should use 1/0 values - please use the BIT
helper for this.
void cave_state::jumbogod_map(address_map &map) | ||
{ | ||
map(0x000000, 0x07ffff).rom(); // ROM | ||
map(0x100000, 0x10ffff).ram().share("nvram"); // RAM (battery) | ||
map(0x200000, 0x20ffff).ram().share(m_spriteram[0]); // Sprites | ||
map(0x300000, 0x307fff).m(m_tilemap[0], FUNC(tilemap038_device::vram_map)); // Layer 0 | ||
map(0x400000, 0x40007f).w(FUNC(cave_state::videoregs_w<0>)).share(m_videoregs[0]); // Video Regs | ||
map(0x400000, 0x400007).r(FUNC(cave_state::irq_cause_r)); // IRQ Cause | ||
map(0x400068, 0x400069).w("watchdog", FUNC(watchdog_timer_device::reset16_w)); // Watchdog | ||
map(0x500000, 0x500005).w(m_tilemap[0], FUNC(tilemap038_device::vregs_w)); // Layer 0 Control | ||
map(0x600000, 0x60ffff).ram().w(m_palette[0], FUNC(palette_device::write16)).share("palette.0"); // Palette | ||
map(0x700000, 0x700001).portr("IN0"); // Inputs + EEPROM | ||
map(0x700002, 0x700003).portr("IN1"); // Inputs | ||
map(0x800001, 0x800001).rw("oki1", FUNC(okim6295_device::read), FUNC(okim6295_device::write)); // M6295 | ||
map(0x900001, 0x900001).rw("oki2", FUNC(okim6295_device::read), FUNC(okim6295_device::write)); // M6295 | ||
map(0xc00001, 0xc00001).w(FUNC(cave_state::jumbogod_leds_w)); // Leds | ||
map(0xe00001, 0xe00001).w(FUNC(cave_state::tjumpman_eeprom_w)); // EEPROM | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is almost identical to pacslot_map
– please call pacslot_map
and then just install the two different handlers you need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any objection to adding oki2
to pacslot_map and consolidating the two? I checked and pacslot seems to initialize the second (unused) OKI during startup at the same address. I'd then just have to install the alternate handler for the lamp I/O.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if pacslot has the extra OKI on board?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the second OKI isn’t populated on pacslot
, it shouldn’t be added. Many games do run code code to initialise unpopulated peripherals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The peripheral itself is present, it just has no ROM data attached. So, it wouldn't make noise properly if commanded, but it would respond to register reads, etc. It is physically on the PCB on the PCB though.
static INPUT_PORTS_START( jumbogod ) | ||
PORT_START("IN0") | ||
PORT_BIT( 0x01, IP_ACTIVE_LOW, IPT_UNKNOWN ) | ||
PORT_BIT( 0x02, IP_ACTIVE_LOW, IPT_COIN1 ) PORT_IMPULSE(10) // credits (impulse needed to coin up reliably) | ||
PORT_BIT( 0x04, IP_ACTIVE_LOW, IPT_UNKNOWN ) | ||
PORT_BIT( 0x08, IP_ACTIVE_HIGH, IPT_CUSTOM ) PORT_READ_LINE_DEVICE_MEMBER("eeprom", eeprom_serial_93cxx_device, do_read) | ||
PORT_BIT( 0x10, IP_ACTIVE_LOW, IPT_BUTTON1 ) PORT_NAME( "Tai Atari (Body Slam)" ) | ||
PORT_BIT( 0x20, IP_ACTIVE_LOW, IPT_BUTTON3 ) PORT_NAME( "Shippo Kougeki (Tail Attack)") // not a mistake; it is #3 by the game's test menu. | ||
PORT_BIT( 0xC0, IP_ACTIVE_LOW, IPT_UNKNOWN ) | ||
|
||
PORT_START("IN1") | ||
PORT_SERVICE( 0x01, IP_ACTIVE_LOW ) // must stay on during service mode | ||
PORT_BIT( 0x06, IP_ACTIVE_LOW, IPT_UNKNOWN ) | ||
PORT_CONFNAME( 0x08, 0x08, "Self Test" ) | ||
PORT_CONFSETTING( 0x08, DEF_STR( Off ) ) | ||
PORT_CONFSETTING( 0x00, DEF_STR( On ) ) | ||
PORT_BIT( 0x10, IP_ACTIVE_LOW, IPT_BUTTON2 ) PORT_NAME( "Nissen Kougeki (Atomic Breath)" ) | ||
PORT_BIT( 0x20, IP_ACTIVE_LOW, IPT_BUTTON4 ) PORT_NAME( "Nissen Kougeki (Atomic Breath)" ) | ||
PORT_BIT( 0xC0, IP_ACTIVE_LOW, IPT_UNKNOWN ) | ||
INPUT_PORTS_END |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s going to confuse users if there are two identically named buttons.
Also, you should use PORT_SERVICE
for the test mode switch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, PORT_SERVICE is already being used for the operator test menu in line with how it seems to be used for many titles. The game has a separate diagnostic self test mode, operated by another switch. Do you mean that they should be switched? In the current configuration, striking F2 brings up the operator menu with settings, which is the behavior I expected to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I missed that PORT_SERVICE
is already being used. Can you use another one of the service controls? Is IPT_SERVICE1
available? For example something like:
PORT_BIT( 0x08, IP_ACTIVE_LOW, IPT_SERVICE1 ) PORT_TOGGLE PORT_NAME( "Self Test" )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do that, thanks - I was not aware of that macro.
New Working Software: Jumbo Godzilla [Toho / Namco]
Jumbo Godzilla runs on hardware nearly identical to Pac-Slot, with the board moniker N-42 EM VIDEO PCB stamped on it.
Notable differences compared to N-44:
All ROM names are based directly upon the labels on the ROMs on the PCB. All GALs were found to be locked and thus remain undumped.
I do not have access to an original cabinet, so I can not 100% verify the accuracy of the lamp and motor control outputs. My PCB has damage, possibly from overcurrent on one of these controls, so at this time it is not something I can use for further testing. Furthermore, I do not have a wiring harness for it, so no clues may be offered there either.
This game has both an operator menu as well as a self-check screen, even though the PCB itself has no DIP switches.