IRF Posted July 15, 2016 Author Report Share Posted July 15, 2016 (edited) 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 September 5, 2016 by IRF jetsetdanny and Spider 2 Quote Link to comment Share on other sites More sharing options...
jetsetdanny Posted July 15, 2016 Report Share Posted July 15, 2016 Thanks for this info, Ian! Quote Link to comment Share on other sites More sharing options...
IRF Posted July 15, 2016 Author Report Share Posted July 15, 2016 (edited) 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 July 16, 2016 by IRF Spider and jetsetdanny 2 Quote Link to comment Share on other sites More sharing options...
IRF Posted September 14, 2016 Author Report Share Posted September 14, 2016 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. jetsetdanny 1 Quote Link to comment Share on other sites More sharing options...
jgharston Posted February 18, 2017 Report Share Posted February 18, 2017 (edited) 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 February 18, 2017 by jgharston IRF, Spider and jetsetdanny 3 Quote Link to comment Share on other sites More sharing options...
IRF Posted February 18, 2017 Author Report Share Posted February 18, 2017 I think that might be quite similar to Richard (SkoolKid)'s fix: http://skoolkid.github.io/jetsetwilly/reference/bugs.html#corruptedConveyors jetsetdanny and Spider 2 Quote Link to comment Share on other sites More sharing options...
jetsetdanny Posted February 22, 2017 Report Share Posted February 22, 2017 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. Spider 1 Quote Link to comment Share on other sites More sharing options...
IRF Posted May 8, 2017 Author Report Share Posted May 8, 2017 (edited) 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 May 8, 2017 by IRF Spider and jetsetdanny 2 Quote Link to comment Share on other sites More sharing options...
jgharston Posted May 9, 2017 Report Share Posted May 9, 2017 (edited) 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 May 9, 2017 by jgharston IRF, Spider and jetsetdanny 3 Quote Link to comment Share on other sites More sharing options...
IRF Posted November 17, 2017 Author Report Share Posted November 17, 2017 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!? Spider and SkoolKid 2 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
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.