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

Modified k052591 PMC emulation to reflect how the real programs work #11992

Merged
merged 2 commits into from Feb 3, 2024

Conversation

furrtek
Copy link
Contributor

@furrtek furrtek commented Feb 1, 2024

Small changes based on silicon reverse-engineering.

spy: Confirmed projection function constants, more accurate collision check without the need for special case handling
thunderx: Simplified collision check, fixed object flags updates
hexion: Added special 16-byte VRAM clearing command

Not sure if mentionning an URL to the documentation in comments is the right thing to do.

spy: Confirmed projection function constants, more accurate collision check without the need for special case handling
thunderx: Simplified collision check, fixed object flags updates
hexion: Added special 16-byte VRAM clearing command
Added comments to PMC program dumps
src/mame/konami/hexion.cpp Outdated Show resolved Hide resolved
src/mame/konami/spy.cpp Outdated Show resolved Hide resolved
src/mame/konami/spy.cpp Outdated Show resolved Hide resolved
src/mame/konami/spy.cpp Outdated Show resolved Hide resolved
Comment on lines 237 to 257
int bank = m_unkram[0] & 1;
memset(m_vram[bank], m_unkram[1], 0x2000);
m_bg_tilemap[bank]->mark_all_dirty();
uint8_t command = m_pmcram[0];
if (command <= 1)
{
memset(m_vram[command], m_pmcram[1], 0x2000);
m_bg_tilemap[command]->mark_all_dirty();
}
else
{
memset(m_vram[m_pmcram[4]] + ((m_pmcram[3] << 8) + m_pmcram[2]), 0, 16);
m_bg_tilemap[m_pmcram[4]]->mark_all_dirty();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’ve removed the mask on the video RAM bank here – that causes a potential heap smash. You should also do a mask or bounds check on the offset when doing the 16-byte clear as there’s nothing to stop it smashing memory off the end of the area the way it is now.

@rb6502 rb6502 merged commit bfdcc04 into mamedev:master Feb 3, 2024
5 checks passed
Comment on lines +256 to +258
uint8_t bank = m_pmcram[4] & 1;
memset(m_vram[bank] + (get_u16le(&m_pmcram[2]) & 0x1fff), 0, 16);
m_bg_tilemap[bank]->mark_all_dirty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s still a potential heap smash here – consider what happens if (get_u16le(&m_pmcram[2]) & 0x1fff) is 0x1ff8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed on my side. Should I submit a new pull request ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already fixed it.

run_collisions(s0, e0, s1, e1, cm, hm);
// set flags
p1[0] = (p1[0] & 0x8f) | 0x10;
if (p1 > (uint8_t*)0xe6) // This address value is hardcoded in the PMC program
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@furrtek this pointer comparison won't work since &m_pmcram[0] is not 0.
If I read your disasm correctly, it's (&p1[4] >= &m_pmcram[0xe6]), but then collisions don't work anymore.

Copy link
Member

@happppp happppp Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe p0[4] instead of p1[4], but then I see another issue where the address value in the US version is $136 instead of $E6,

Copy link
Member

@happppp happppp Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be ok with this: 45f7cab
small update 1fb0abe (0x136 instead of 0x36 for the other set)

Mokona pushed a commit to Mokona/mame that referenced this pull request Feb 28, 2024
…amedev#11992)

* Modified k052591 PMC emulation to reflect how the real programs work
* spy: Confirmed projection function constants, more accurate collision check without the need for special case handling
* thunderx: Simplified collision check, fixed object flags updates
* hexion: Added special 16-byte VRAM clearing command
* Added comments to PMC program dumps
* Use multibyte.h functions, variable scope and type cleanup
stonedDiscord pushed a commit to stonedDiscord/mame that referenced this pull request Apr 8, 2024
…amedev#11992)

* Modified k052591 PMC emulation to reflect how the real programs work
* spy: Confirmed projection function constants, more accurate collision check without the need for special case handling
* thunderx: Simplified collision check, fixed object flags updates
* hexion: Added special 16-byte VRAM clearing command
* Added comments to PMC program dumps
* Use multibyte.h functions, variable scope and type cleanup
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 this pull request may close these issues.

None yet

4 participants