Skip to content

Conversation

@mlund
Copy link
Contributor

@mlund mlund commented Aug 24, 2024

This PR brings non-breaking changes to the Commander X16 target. Apologies for the combo of changes; let me know if I should split to more focused parts.

  • Bug fix: leaf attributes accidentally inside function args.
  • Bug fix: Function get_ostype() did not work.
  • Bug fix: Function vpoke() had an endianness bug.
  • Convert several KERNAL wrapper functions to inline asm, saving around 10-20 bytes per function.
    - cx16_k_screen_set_charset()
    - vpeek()
    - vpoke()
    - waitsync()
  • Add unique assembler section names and progbits to all cbm_k_ functions.
  • Removed superfluous ldx #0 before returning unsigned char (leftover from cc65).
  • Add explicit enum variant type.
  • Add missing emulator registers.
  • Minor changes to code comments.
  • Add labels for ROM banks as specified in the cx16-docs.
  • Add struct for VERA PSG audio channels.
  • Add assembler macro for JSRFAR access (KERNAL routine).
  • Add volatile access to KERNAL_VERSION register.
  • Use typedef struct {...} instead of struct {} for JoyState. This is better practice and implicit in C++.
  • Add CBM_LOAD_* enum for controlling KERNAL function LOAD. CX16 has extended functionality.
  • Compile-time size checks of some structs using static_assert. Guarded for C23/C++11 or higher.

@mlund mlund marked this pull request as draft August 26, 2024 06:05
@mysterymath
Copy link
Member

mysterymath commented Aug 26, 2024

This is a bit much for one PR; it'll take me a long time to review everything, which will block the easier stuff from getting in. (It's really nice to see a simple change in a PR and have it be so obviously correct that I can immediately just hit the button. Even if I need to do it 20 times, that's still preferable.)

Would you mind:

  • Splitting out a PR for any NFC changes (cosmetics, code cleanliness refactoring, comments, etc)
  • Splitting the other changes out by "topic"

@mlund mlund closed this Aug 26, 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

Successfully merging this pull request may close these issues.

2 participants