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

parse VS system #747

Merged
merged 2 commits into from
Jan 13, 2022
Merged

parse VS system #747

merged 2 commits into from
Jan 13, 2022

Conversation

CasualPokePlayer
Copy link
Collaborator

Leaving this as draft as I don't know if it's settled on whether to marked these as VS or Arcade. Also a db update would be needed for a VS id it seems.

@vadosnaprimer
Copy link
Collaborator

What needs to be decided is whether we make VS a new system on the site, or just a game version. Since VS ROMs seems to be just NES roms (and it decides how to load them using iNES header?), leaving them as game versions seems logical to me.

@CasualPokePlayer
Copy link
Collaborator Author

CasualPokePlayer commented Jan 3, 2022

What needs to be decided is whether we make VS a new system on the site, or just a game version. Since VS ROMs seems to be just NES roms (and it decides how to load them using iNES header?), leaving them as game versions seems logical to me.

What makes an NES ROM an NES ROM is that it is a ROM image that came from an NES cart (or could be put on an NES cart and loaded correctly). VS ROMs are definitely not that. iNES header just has info like PRG and CHR size which are needed to be provided for the emulator to properly use the ROM (which otherwise need some gamedb to provide them), and are identical to the ROMs in the VS arcade systems after just removing the header.

@CasualPokePlayer
Copy link
Collaborator Author

CasualPokePlayer commented Jan 3, 2022

So here's an interesting monkey wrench for this debate;
image
image
As the VS system is an Arcade you could use MAME-RR for TASing games on that system. I don't know why you would, but you can. And whenever MAMEHawk is finished that will be an option too. Perhaps just marking VS TASes on NESHawk as Arcade is the best way to go here.

@meshuggahtas
Copy link
Collaborator

  • Should distinguish VS and PC10 (Playchoice-10) from NES and Arcade
  • I advise adding new platforms from the parent platform. NES-VS and NES-PC10. Might be useful for other platforms as well. This also makes writing rules easier to refelct on a group of NES games that aren't NES games (referring to arcade rules that doesn't happens on home console NES games)
  • Obvious but here it is, VS and PC10 should be tased with NESHawk. In sense of TASing, those VS and PC10 ROM structures and technical details matches NES better than MAMEHawk.

@vadosnaprimer
Copy link
Collaborator

Okay yes, VS. games work in MAME just fine. Some have dual screen (Baseball and Tennis). It's true that VS games have VS in their name already. If somebody plays them in MAME-RR or MAMEHawk, the system will be parsed as Arcade, and I don't think this should be treated as wrong.

Dang every option feels right to some degree. @adelikat do you have a preference?

@adelikat
Copy link
Collaborator

adelikat commented Jan 3, 2022

We need to add the VS system Id first.
Need Unit Tests to approve.

@SamsaraTAS
Copy link
Collaborator

My only major preference here is that VS TASes done on any emulator (NESHawk, FCEUX, MAMEHawk, etc) should always have the same ID, whatever it ends up being. I can go either way on that, with a slight preference for VS or NES-VS, though I'd imagine that would lead to a lot of fixing "NES-VS VS Super Mario Bros" in submissions. Occam's razor here is probably to parse it as Arcade, and I wouldn't argue against that if there's more of a consensus for it.

@vadosnaprimer
Copy link
Collaborator

Yeah I feel if we don't put it as Arcade and somebody TASes it on MAMEHawk, we'd have to tweak the parser as well as MAMEHawk movie header logic quite a bit, which in turn feels like adding complexity where it's not required.

@CasualPokePlayer
Copy link
Collaborator Author

CasualPokePlayer commented Jan 8, 2022

There seems to be releative agreement on using Arcade for VS TASes? Which case this PR can continue (tests are in already, and VS system ID doesn't need to be added).

@CasualPokePlayer CasualPokePlayer marked this pull request as ready for review January 8, 2022 07:10
@vadosnaprimer
Copy link
Collaborator

This should be good to merge I think.

@adelikat
Copy link
Collaborator

It isn't good to merge because we don't have a VS system id, so this would return an unrecognized code that would then crash the submission/userfile upload process

@vadosnaprimer
Copy link
Collaborator

It's Arcade.

@CasualPokePlayer
Copy link
Collaborator Author

CasualPokePlayer commented Jan 13, 2022

It's set to change the system ID to Arcade and override framerate with NES framerate. A test is included for this case.

@adelikat
Copy link
Collaborator

I missed that somehow

@adelikat adelikat merged commit ae4e19b into TASVideos:main Jan 13, 2022
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.

5 participants