Jump to content
Jet Set Willy & Manic Miner Community

Pokes for Fixing the Cell Graphics Bug in JSW


IRF

Recommended Posts

Looking at the 'Draw the current room to the screen buffer' code, in association with Stuart's POKEs, I believe that the single byte in the existing code at #8D4D is superfluous with Stuart's fix in place, and could be consolidated away.  Its original purpose was to pass the value of C on to E, whilst the CPIR loop worked on the BC register pair.

 

Stuart's code (as an alternative to the CPIR loop that yielded the Cell Graphics Bug in the original game) uses the DE register-pair to count (and therefore bypass) the intervening graphic bytes between each attribute byte.  It also uses the B register, but not the C register.

 

After an attribute byte match has been found, the program returns back to #8D59, where an identical instruction to that at #8D4D is now located.  This is instead of the command in the original code that restores the value of E back to C; the value of C no longer needs to be restored, but the value of E does, and so an LD E, C command is applied - again - and this supercedes the need for the first attempt at passing C's value to E (#8D4D).

 

For reference:

http://jswmm.co.uk/topic/93-new-jsw-build-suggestions/?p=798

Edited by IRF
Link to comment
Share on other sites

Thanks for this info, Ian!

Further to the above, I just NOPped out the single byte at #8D4D (or the equivalent location) in a test file that I created a while back, in which I'd integrated Stuart's Cell-Graphics Bug Fix into the main 'Drawn the current room...' routine (it's the first of two '59' entries in the hex code in the vicinity), and Stuart's patch still works just fine.

 

So to quote Richard (SkoolKid), #8D4D is a 'redundant instruction' once the patch is in place.

 

And furthermore, the spare byte is in a potentially useful place, as it isn't too far away from the start of the routine in which it sits (which in turn is preceded by the Game Over sequence, which in our recent projects has been amended/relocated elsewhere, leaving a good chunk of unused code in the vicinity to consolidate the extra byte with).

Edited by IRF
Link to comment
Share on other sites

  • 1 month later...

Further to the above, I've just noticed that if the redundant LD E, C instruction that (used to be) at #8D4D is consolidated away, rather than just being NOPped out, then the relative jump that (used to be) at #8D67 should have its operand adjusted accordingly (from '20 DD' to 20 DE'), so that it loops back to the LD A, (IX+$00) command that (was formerly) at #8D4E.

Link to comment
Share on other sites

  • 5 months later...

I was doing some tidying up of my JSW documents a couple of days ago and was looking at the block graphics drawing code. Not being able to remember seeing a bugfix for it anywhere I thought I'd test a fix I'd had sitting at the back of my mind for years. I tested it yesterday, and with one tweek this is it. See also link.

Original code:                Change to:L8D4B: LD   C,&00             L8D4B: 1E 00     LD   E,&00           ; Start at screen address 0L8D4D: LD   E,C               L8D4D: DD 7E 00  LD   A,(IX+0)        ; Get current attribute       LD   A,(IX+&00)               21 97 80  LD   HL,BACKGROUND-9 ; Start at the background attribute       LD   HL,BACKGROUND            01 09 00  LD   BC,9            ; Nine bytes per block       LD   BC,&0036          L8D56: 09        ADD  HL,BC           ; Step to next block       CPIR                          BE        CP   (HL)            ; Does attribute byte match?       LD   C,E                      20 FB     JR   NZ,L8D56        ; Check next block       LD   B,&08                              LD   B,&08           ; Eight pixel-linesL8D5C: LD   D,&00             L8D5C:           LD   D,&00           ; High byte of screen bitmap address                              L8D5E: 23        INC  HL              ; Point to bitmapL8D5E: LD   A,(HL)                   7E        LD   A,(HL)          ; Get a character byte       LD   (DE),A                   12        LD   (DE),A          ; Store into screen buffer       INC  HL       INC  D                                  INC  D               ; Move to next pixel-line       DJNZ L8D5E                              DJNZ L8D5E           ; Loop for eight pixel-lines       INC  IX                                 INC  IX              ; Move to next attribute       INC  C                        1C        INC  E               ; Move to next screen address       JP   NZ,L8D4D                           JP   NZ,L8D4D        ; Loop for 256 attributes       RET                                     RETThe following hex patchfile will apply this fix::0F8D4B001E00DD7E0021978001090009BE20FB7C:098D5E00237E121410FADD231C1F:0000000000
Edited by jgharston
Link to comment
Share on other sites

I think that might be quite similar to Richard (SkoolKid)'s fix:

 

http://skoolkid.github.io/jetsetwilly/reference/bugs.html#corruptedConveyors

 

Jonathan (J. G. Harston) is acknowledged in the Credits section of SkoolKid's disassembly. Since I believe his fix has been available online for a long time (before SkoolKid's otherwise great disassembly was created), if the two solutions are similar (or the same, perhaps?), I wouldn't be surprised if Richard (SkoolKid) had actually copied Jonathan's solution.

Link to comment
Share on other sites

  • 2 months later...

 

I was doing some tidying up of my JSW documents a couple of days ago and was looking at the block graphics drawing code. Not being able to remember seeing a bugfix for it anywhere I thought I'd test a fix I'd had sitting at the back of my mind for years. I tested it yesterday, and with one tweek this is it. See also link.

Original code:                Change to:L8D4B: LD   C,&00             L8D4B: 1E 00     LD   E,&00           ; Start at screen address 0L8D4D: LD   E,C               L8D4D: DD 7E 00  LD   A,(IX+0)        ; Get current attribute       LD   A,(IX+&00)               21 97 80  LD   HL,BACKGROUND-9 ; Start at the background attribute       LD   HL,BACKGROUND            01 09 00  LD   BC,9            ; Nine bytes per block       LD   BC,&0036          L8D56: 09        ADD  HL,BC           ; Step to next block       CPIR                          BE        CP   (HL)            ; Does attribute byte match?       LD   C,E                      20 FB     JR   NZ,L8D56        ; Check next block       LD   B,&08                              LD   B,&08           ; Eight pixel-linesL8D5C: LD   D,&00             L8D5C:           LD   D,&00           ; High byte of screen bitmap address                              L8D5E: 23        INC  HL              ; Point to bitmapL8D5E: LD   A,(HL)                   7E        LD   A,(HL)          ; Get a character byte       LD   (DE),A                   12        LD   (DE),A          ; Store into screen buffer       INC  HL       INC  D                                  INC  D               ; Move to next pixel-line       DJNZ L8D5E                              DJNZ L8D5E           ; Loop for eight pixel-lines       INC  IX                                 INC  IX              ; Move to next attribute       INC  C                        1C        INC  E               ; Move to next screen address       JP   NZ,L8D4D                           JP   NZ,L8D4D        ; Loop for 256 attributes       RET                                     RETThe following hex patchfile will apply this fix::0F8D4B001E00DD7E0021978001090009BE20FB7C:098D5E00237E121410FADD231C1F:0000000000

 

Shouldn't the operand in bold above actually be FC, to point back to #8D56?

 

Although as it happens, the value of FB as stated above just jumps back to a '00' operand byte that sits immediately prior to the correct destination address.  This is treated as a NOP, and so has no ill effect other than to (imperceptibly) slow down the routine.

 

 

Jonathan (J. G. Harston) is acknowledged in the Credits section of SkoolKid's disassembly. Since I believe his fix has been available online for a long time (before SkoolKid's otherwise great disassembly was created), if the two solutions are similar (or the same, perhaps?), I wouldn't be surprised if Richard (SkoolKid) had actually copied Jonathan's solution.

 
I believe that they are similar solutions, but Richard optimised the patch so as to fit within his self-imposed limit of a maximum of ten POKES per 'fix'.  In order to achieve this, he identified and recycled an existing relative jump of the same length as the one I referred to above (i.e. '20 FC') at #FF28 within the unused TRS-DOS code at the end of the game file.
 
(In fact, it was studying Richard's solution, in conjunction with his disassembly, which led me to spot Jonathan's apparent error.)
Edited by IRF
Link to comment
Share on other sites

You're right, the branch offset should be FC. I think I had the ADD somewhere else initially starting at BACKGROUND instead of BACKGROUND-9. I edited the bytes in manually and it worked correctly, I obviously missed the disassembly then saying JR 8D55 instead of JR 8556. I'll tweek my patch on my site.

Edited by jgharston
Link to comment
Share on other sites

  • 6 months later...

Has anyone noticed the instance of the Cell Graphics Bug affecting the Fire cells in 'A Bit of Tree' in the original JSW?

 

It isn't documented anywhere in SkoolKid's JSW disassembly, I don't recall Stuart Brady mentioning it when he introduced his Cell Graphics Bug Fix, and a search in the Yahoo Group has drawn a blank.

 

However, I find it hard to believe (in fact incredible!) that this hasn't been spotted before!?

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.