Jump to content


Photo

Pokes for Fixing the Cell Graphics Bug in JSW


  • Please log in to reply
45 replies to this topic

#11 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 15 July 2016 - 06:25 PM

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/t...gestions/?p=798


Edited by IRF, 05 September 2016 - 12:39 PM.


#12 jetsetdanny

jetsetdanny

    Advanced Member

  • Contributor
  • 1,895 posts

Posted 15 July 2016 - 06:43 PM

Thanks for this info, Ian!



#13 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 15 July 2016 - 08:19 PM

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, 16 July 2016 - 11:26 AM.


#14 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 14 September 2016 - 10:34 PM

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.



#15 jgharston

jgharston

    JGH

  • Contributor
  • 17 posts

Posted 18 February 2017 - 07:08 PM

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 0
L8D4D: 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-lines
L8D5C: LD   D,&00             L8D5C:           LD   D,&00           ; High byte of screen bitmap address
                              L8D5E: 23        INC  HL              ; Point to bitmap
L8D5E: 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                                     RET

The following hex patchfile will apply this fix:

:0F8D4B001E00DD7E0021978001090009BE20FB7C
:098D5E00237E121410FADD231C1F
:0000000000

Edited by jgharston, 18 February 2017 - 07:12 PM.


#16 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 18 February 2017 - 07:14 PM

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

 

http://skoolkid.gith...ruptedConveyors



#17 jetsetdanny

jetsetdanny

    Advanced Member

  • Contributor
  • 1,895 posts

Posted 22 February 2017 - 11:05 PM

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

 

http://skoolkid.gith...ruptedConveyors

 

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.



#18 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 08 May 2017 - 10:19 PM

 

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 0
L8D4D: 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-lines
L8D5C: LD   D,&00             L8D5C:           LD   D,&00           ; High byte of screen bitmap address
                              L8D5E: 23        INC  HL              ; Point to bitmap
L8D5E: 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                                     RET

The 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, 08 May 2017 - 10:44 PM.


#19 jgharston

jgharston

    JGH

  • Contributor
  • 17 posts

Posted 09 May 2017 - 02:12 PM

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, 09 May 2017 - 02:16 PM.


#20 IRF

IRF

    Advanced Member

  • Contributor
  • 3,760 posts

Posted 17 November 2017 - 01:19 AM

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!?






0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users