remy / vscode-nextbasic

VS Code tools for NextBASIC
https://marketplace.visualstudio.com/items?itemName=remysharp.nextbasic
9 stars 2 forks source link

Corruption of Sprites and Maps #13

Closed StevenPowley closed 4 years ago

StevenPowley commented 4 years ago

When transferring to CSPECT the first run on files that have included .spr or .map files (raw data) work fine. However subsequent runs appear to overwrite and corrupt the data. For example using the sprite features basic demo from the next distribution along with its' associated dksprite.spr file from the environment to CSPECT will run fine the first (or first few) times but after repeated use will corrupt and show garbage on main character sprite.

remy commented 4 years ago

This might be tricky to debug, but let's give it a go.

First thing to rule out:

Are you using sprites that are already inside the Next image (that cspect is using), or are you transferring them across upon each run from VS Code?

Can you provide a pared down example of code that you're able to replicate this with?

StevenPowley commented 4 years ago

Remy, Yeah I can appreciate the difficulty with this one. To summarise how I got to the situation..I copied from the latest next image the spritefeatures.bas and also the dksprites.spr (I used HDFMGOOEY) but same results from hdfmonkeyI can verify that the sprite file is good by checking it with your online tools – so at this point – no corruption.I then made some edits and paired back the bas file and then tested in CSPECT directly from VS Code The paired down code is as follows:

10 run at 1 15 PROC initSprites() 20 PROC initCharacters() 30 PROC initScreen() 40 LET %c=0:%v=2 50 ; 60 REPEAT 70 IF INKEY$ ="q" THEN PROC moveChar(0,%-v, BIN 0111) 80 IF INKEY$ ="a" THEN PROC moveChar(0,%v, BIN 0011) 90 IF INKEY$ ="o" THEN PROC moveChar(%-v,0, BIN 1001) 100 IF INKEY$ ="p" THEN PROC moveChar(%v,0, BIN 0001) 220 REPEAT UNTIL 0 230 ; 240 DEFPROC moveChar(%m,%n,%f) 250 IF % SGN {((x+m) < 0) OR ((x+m) > 304)} THEN ENDPROC 260 IF % SGN {((y+n) < 0) OR ((y+n) > 240)} THEN ENDPROC 280 LET %x=%x+m 290 LET %y=%y+n 300 LET %a(c)=%a(c)+1: IF %a(c) > f(c) THEN LET %a(c)=0 310 SPRITE %c,%x,%y,%i(c)+a(c),%f 340 ENDPROC 840 ; 850 DEFPROC initSprites() 860 SPRITE CLEAR 865 rem LOAD "c:/DEMOS/NextBASIC/basicSprites/DKSprite.spr" BANK 12 870 LOAD "DKSprite.spr" BANK 12 880 SPRITE BANK 12 890 ENDPROC 900 ; 910 DEFPROC initCharacters() 950 LET %x=0: LET %y=240 960 REM Animation frames 0..3 970 LET %a(0)=0: LET %f(0)=3 980 REM Image for first frame 990 LET %i(0)=1 1030 SPRITE 0,%x,%y,%i(0),1 1390 ENDPROC 1400 ; 1410 DEFPROC initScreen() 1420 LAYER CLEAR 1430 PAPER 7: INK 0: BORDER 4: CLS 1440 SPRITE PRINT 1: SPRITE BORDER 1 1450 POKE 23658,0: REM turn off CAPS LOCK 1470 PRINT INVERSE 1;"Q"; INVERSE 0;" "; INVERSE 1;"A"; INVERSE 0;" "; INVERSE 1;"O"; INVERSE 0;" "; INVERSE 1;"P"; INVERSE 0;" move character" 1560 ENDPROC 1570 ;

Both this and the dksprite.spr file end up in devel on the cspect image. The corruption appears to be at the beginning of the sprite file as it starts impacting the first sprite pattern. DKSprite.zip Even after running just once the sprite file corrupted and the outcome from the cspect image was copied back to the desktop and check in your sprite tool again (corrupted sprite file copy attached) the images are clearly damaged.Hope that helps a little.. I will redirect the basic to use the sprites already in the demo folder instead of the one transferred over. 

StevenPowley commented 4 years ago

Having just tested by loading the default file from the image instead of the one being transferred – I can confirm this file does not corrupt – so it is transfer related somehow...  870 LOAD "c:/DEMOS/NextBASIC/basicSprites/DKSprite.spr" BANK 12  

remy commented 4 years ago

Ah… I think the email reply is corrupting your attachments!

StevenPowley commented 4 years ago

oh - looks awful - let's try that again... edited now on site and hopefully more readable. Sorry for that - first time using github tbh.

remy commented 4 years ago

No problem 😊

So I have a suspicion that it's the hdfmonkey file that comes with hdfmgooy that might be the problem - I saw something with another windows user who used hdfmgooy and something very weird was going on.

In the readme for the extension, there's a link to a specially patched version of hdfmonkey (I'm on my mobile right now so can't easily get the link), but can you download that version and point the vscode extension settings to it, then repeat your test?

StevenPowley commented 4 years ago

I downloaded the windows version of hdfmonkey you link on the extension page. Completely removed previous hdfmonkey and hdfmgooey. Using the new version and updating the path accordingly has made no impact.

remy commented 4 years ago

Okay - thanks for testing that, at least it rules it out.

So that I can confirm I've got your setup right in my head, can you correct me if this is wrong anywhere.

For reference: your pc refers to the machine running VS Code, cspect refers to the the Next machine emulator and the virtual disk image.

  1. You've copied two files from cspect on your your pc: spritefeatures.bas and dksprites.spr.
  2. From that directory that contain the two files on your pc, you have VS Code open and you run the 'NextBASIC: run with cspect' command
  3. That autostarts spritefeatures.bas and works, the first time only (want to check this part is right, or is it that it never works when sending from VS Code)
  4. Repeating 'NextBASIC: run with cspect' command results in corruption

From the zip file of the .spr file I can see that, somehow, the NextBASIC has ended up inside of the sprite file:

Screenshot 2020-06-04 at 15 51 55

It looks like line numbers 1110 to around 1220 - which doesn't match anything you pasted yesterday so I'm not quite sure where it's coming from (in fact, the NextBASIC isn't even encoded, so it's like plain text has gotten into it somehow).

Would you be able to provide a screenshot of the stages you're following or better yet a video/screencast of what you're doing? I feel like there's something else at play here that's causing the corruption (certainly I've never had it myself nor the other folk using this extension). From the code point of view, there's only one place I can think it could potentially happen and that's if a file overwrote another file - but it would require the same filename… on that train of though, could you provide a screenshot of the directory you're working in (the one containing dksprites.spr) on your PC?

StevenPowley commented 4 years ago

Interesting that the DKSprite.spr dump shows a basic file which is part of something else I was experimenting with and does exist as a file in the same folder as the others but with the filename test2.bas and has never had a different filename and never been in the code of spritefeatures.bas. You are correct in the assumptions on my naming of machines. spritefeatures.bas does autostart and runs showing the DKSprites.spr sprites almost as expected but the moving sprite is corrupt immediately from frame 0 from first run and subsequent. The corruption does not appear to be consistent - in as much as sometimes it impacts the first frame and sometimes more. I am wondering if the corruption gets worse as I make edits in VS Code and then launch again into CSpect with the edits so will test that and see if I can confirm this creeping corruption theory. files in folder

remy commented 4 years ago

Another thing you could try, that would confirm it's definitely happening in the extension is this:

  1. Using the extension, run the NextBASIC: export to .bas command on your code
  2. Using hdfmgooy, send your sprite and the exported .bas file across to cspect
  3. Run your tests on your program

This would assume no corruption (though might be worth double checking your source spr file is clean by dropping into the sprite tool here: https://zx.remysharp.com/sprites/). If that's the case, then at least I know where to look in the extension code (though I had to admit it's definitely puzzling!)

StevenPowley commented 4 years ago

Attempting to test the creeping corruption theory I noticed that sometimes it would run without sprite corruption, but could not establish a precise trigger pattern. It would run 8-10 times in a row and be fine on one launch, on a subsequent test I would open VS Code launch the basic with CSpect and it would be corrupt instantly. I've also noticed now checking with a local hex editor that the corruption can also come from the other basic file in the same folder (exported.bas) not just the test2.bas which you noticed in your hex dump. This alternative basic file is a longer file and that seems to account for the difference in extent/lengths of the corruption.

StevenPowley commented 4 years ago

Remy - followed your guidance.

Using the extension, run the NextBASIC: export to .bas command on your codeUsing hdfmgooy, send your sprite and the exported .bas file across to cspectRun your tests on your program

Doing this and running delivered full operation without corruption. Repeating the process form start on 5 attempts all provided the same outcome - full operation without corruption.

Hope this helps a little.

remy commented 4 years ago

This helps, in that it helps point back to the extension. Still scratching my head…

Andy1966uk commented 4 years ago

I have seen this to. I put it down to CSPECT (no proof though) I just every so often delete the files that have been copied over, and this cures it, until it happens again... so Steven isn't alone....

remy commented 4 years ago

This is useful to know. I'm looking to see if there's any possible way there's corruption going on before the send.

I do have one idea, but just on my phone at the moment, so will have to wait until I'm back at the machine before I can help. I'll ping today.

remy commented 4 years ago

So assuming that the tempdir on your machines is c:\windows\temp - the extension puts the original generated file in that folder under the name index.bas - when the file is corrupted inside cspect, can you then check the version in the tmp directory and compare the files (and if you can't, zip up a copy of the two files - one from tmp and one from cspect image and I can take a look)

StevenPowley commented 4 years ago

The temporary directory for me under Windows 10 is %USERPROFILE%\AppData\Local\Temp\ in which I see the basic file (index.bas - nothing else) populate in after the build and run in cspect command is issued in VS Code. I do not see any DKSprite.spr file in this temporary location. I have attached examples under various circumstances. I am not sure I understand the process of conversion and transfer here - are index.bas and spritefeatures.bas created and transferred each time? Where does the DKSprites.spr file come from if not in the temporary directory? Is there a way to ensure no PRODOS header is added during transfer? Examples.zip

remy commented 4 years ago

The process is this:

  1. mkdir /devel on image using hdfmonkey
  2. Put autoexec-pre-compiled.bas that's contained in the extension directory in /nextzxos/autoexec.bas (this is the source nothing special - apart from a big commented out block) - this step is also used to try to work out if there's any issues with hdfmonkey or the cspect image - mostly simple litmus tests.
  3. Find the directory name of your current working file, and if that directory is not . (which IIRC actually just means it's not saved yet), then all files and directories in the working directory are sent using hdfmonkey (ignoring files that start with . - hidden files, and ignoring file paths with spaces - just because I've not sussed out the encoding on windows yet) _(this is where your sprites and support files are copied)
  4. Now convert the source working text into NextBASIC binary and save to $TEMP/index.bas on your local machine
  5. Finally, copy the file $TEMP/index.bas to /devel/index.bas using hdfmonkey.

The way this entire process is designed to work is that it waits for the previous step to complete before moving forward - or if a step fails, then the whole process is then aborted (though potentially leaving a half complete job).

The only place I can (now) see that has any parallelism is the contents of the working directory sent - it'll run the files across in parallel - which might be where the corruption is happening…

I'm going to push a (tested!) change this evening (if I have time - my machine self-sleeps at 10pm!), otherwise tomorrow to move the directory send as sequential. I'll ping here when it's going up.

remy commented 4 years ago

Alrighty, a version is going up now, live in a few minutes on the vscode marketplace. Please give that a try too. I've given some tests locally - I think it might be useful (as I did) to remove your /devel folder just to start with a clean slate.

Let's see how we do.

StevenPowley commented 4 years ago

Tested the new version and after running 20 run in cspect attempts on the previously problematic code there were no errors to be seen in the sprite file. I went on to try my main game that is crawling through development but has a lot of bank loading maps and sprites and that worked perfectly also. It looks like the issue has gone from these initial tests. I will give it a more thorough test later but confidence is high. Also great to see the sprite definition tidied and that annoying integer syntax blip for SGN fixed :-) nice work Remy. Thankyou.

remy commented 4 years ago

That's great to hear - phew, this one had me stumped (until you asked what the process was, I then did the rubber duck process of walking it through and spotted the potential race condition - which it turned out to be).

On any syntax errors that are wrong, if you spot any, just file an issue with the line of code that's causing the problem, and I'll get it fixed. There's around 110 tests I run for this stuff so if it breaks once, it only breaks once :)

remy commented 4 years ago

Closing (in then anticipation that it's all good).