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

Fix issue #187 (and upgrade jest) #188

Closed iflan closed 1 year ago

iflan commented 1 year ago

Before, mmu.ts and cards/langcard.ts would deactivate writing to the language card if prewrite was reset. However, it is totally possible to reset prewrite and leave writing enabled, as shown my Sather in Understanding the Apple II, table 5.4, p. 5-30, and Understanding the Apple IIe, table 5.5, p. 5-24. For example:

    sta  $c08a  ; WRITE DISABLE; READ DISABLE
    lda  $c08b  ; PRE-WRITE set
    lda  $c08b  ; WRITE enabled
    sta  $c08a  ; PRE-WRITE reset; WRITE still enabled!
    lda  $c08b  ; PRE-WRITE set; WRITE still enabled!

would not work correctly because the last line would clear _writebsr before setting _prewrite, which is incorrect.

Now, _writebsr is only set when _prewrite is set and thus only cleared when writeSwitch is false.

There is also a new test for the MMU and LanuageCard classes.

whscullin commented 1 year ago

Sorry I haven't had a chance to look more closely at this, but one thing to consider is that the //e language card logic is a clone of the the code in cards/language, so it might be necessary to update that too? Possibly we should consolidate that logic somewhere?

iflan commented 1 year ago

Good point. There is code in cards/langcard.ts that does the same thing and has the same bug. I'll try to fix that and add a test as well. It's likely that we can consolidate the logic, but I'll do that in another future change.

iflan commented 1 year ago

OK—the latest commit fixes the language card on the Apple II as well. In both cases, the test that repros the bug report in #187 fails unless the fix is applied.

whscullin commented 1 year ago

Weirdly, this works with npm 8.x, (which I had on my machine) but fails with npm 9.x (which is on the machine hosting scullinsteel.com):

pm ERR! Could not resolve dependency:
npm ERR! peer jest@">=20 <=27" from jest-image-snapshot@4.5.1
npm ERR! node_modules/jest-image-snapshot
npm ERR!   dev jest-image-snapshot@"^4.5.1" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: jest@27.5.1
npm ERR! node_modules/jest
npm ERR!   peer jest@">=20 <=27" from jest-image-snapshot@4.5.1
npm ERR!   node_modules/jest-image-snapshot
npm ERR!     dev jest-image-snapshot@"^4.5.1" from the root project

I will try to update jest-image-snapshot and see what happens.

whscullin commented 1 year ago

Updating jest-image-snapshot resolves that. We were a couple major versions behind but I did't encounter any issues updating.