search

whscullin / apple2js

An Apple II emulator originally written in Javascript, now being converted to TypeScript
http://www.scullinsteel.com/apple2/
MIT License
438 stars 57 forks source link

Make `DiskII.drives` a `Record` instead of an array #154

Closed iflan closed 2 years ago

iflan commented 2 years ago

Before, the drives field was an array[0..1] of Drive, but all of the methods took a DriveNumber, which was [1..2]. This meant that code everywhere was always subtracting 1 from the drive number.

Now, drives is a Record<DriveNumber, Drive>, which means tha it has indexes 1, 2 and there's no need to subtract 1 everywhere.

This change updates the DiskII class and its tests.

The motivation for this change is to slowly split the WOZ disk implementation from the nibble disk implementation. I've tried twice, but the change has always grown too big and hairy, so I'm starting very small this time and working my way up.

iflan commented 2 years ago

I've got https://github.com/whscullin/apple2js/commit/ef71dd77b7babb646418c6e110c14909925ff0cc as a follow-on to this. I'll make a PR when this one is merged (or rejected).

iflan commented 2 years ago

I've now got https://github.com/whscullin/apple2js/commit/1dab93612f84d9d2bee13429b9b74b47e94516a3, too. If you'd like me to just pile these all into one change, let me know. I'm keeping them separate so they are easier to review and test.

iflan commented 2 years ago

I've made a branch with all the changes so you can see what's coming: https://github.com/whscullin/apple2js/compare/main...iflan:apple2js:disk-refactoring

whscullin commented 2 years ago

This LGTM, I'm currently bogged down in other projects and haven't had a chance to play with it but I think the direction is great.

iflan commented 2 years ago

Not a worry. I've just had some free time. There is no rush. :-)