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

Number of elements in the QUIRKS array does not match #18

Closed
salvacam opened this issue Feb 28, 2024 · 4 comments
Closed

Number of elements in the QUIRKS array does not match #18

salvacam opened this issue Feb 28, 2024 · 4 comments

Comments

@salvacam
Copy link
Contributor

The number of elements in the quirks array does not match.

In the file include/chip.h is defined the constant NUM_QUIRKS with value 10
https://github.com/kurtjd/jaxe/blob/main/include/chip8.h#L32C9-L32C19

In this file is comment each elemente of array
https://github.com/kurtjd/jaxe/blob/main/include/chip8.h#L136
but only 9 is comment

I don't know if the value of NUM_QUIRKS should be 9 or 10

In the file src/main.c is defined the array quirks with 10 elements
https://github.com/kurtjd/jaxe/blob/main/src/main.c#L41
but in the src/libretro.c the array quirks with 9 elements
https://github.com/kurtjd/jaxe/blob/main/src/libretro.c#L256

This does not affect performance or generate errors, neither in the SDL version nor in Retroarch, but I think the number of array elements in the two versions should match.

@kurtjd
Copy link
Owner

kurtjd commented Feb 29, 2024

Ah good catch! The correct value should be 10. I discovered the 10th quirk while running a test ROM and then implemented it, but forgot to update that comment and the libretro port.

So there are 10 quirks total (0 - 9 inclusive). The quirk missing from that comment is:

-9: Disable undefined VF after logical OR, AND, XOR (VF is set to 0 with this disabled)

If you'd like, you can update the comment and the libretro port (I'm not too familiar with the inner-workings of the port myself) and I'll gladly accept it. If not I'll definitely update the comment to avoid confusion.

Thanks for pointing it out!

@salvacam
Copy link
Contributor Author

Without a problem, I add the comment and update the libretro port to request the PR.
When I make the PR I will comment here

@salvacam
Copy link
Contributor Author

@kurtjd
Copy link
Owner

kurtjd commented Feb 29, 2024

Merged, appreciate it!

@kurtjd kurtjd closed this as completed Feb 29, 2024
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

No branches or pull requests

2 participants