remy / vscode-nextbasic

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

#bank sections and index.bas not always building with RUNinCSpect v1.11.1 #47

Closed NealeTools closed 4 months ago

NealeTools commented 4 months ago

Hi Remy - sorry...another bug (for me). Sometimes the files with #bank directives don't seem to build. For example: Initially, RUN in CSpect just kept loading a pre-exisitng old index.bas. Eventually, after tying to hash out the #bank part, and then readd it, I got it working with a simple demo with one added #BANK directive. Just now I added a second #bank BANK21, but now it just keeps loading up the original .bas Any ideas? I've saved the file, but it seems to make no difference.

1st Example:

image

Modified:

image

Now if I hash out those #bank directives, it finally loads up the file (that must have been made from a prior run?)

image

I then tried adding a third #bank and this happened:

image

So I hashed out the final #bank:

image

This ought to crash on line 37 because BANK 22 isn't defined, but it just doesn't go there. It isn't loading the BASIC with line 37 in it:

image

Now, it finally works again (after unhashing):

image

Any idea what I am doing wrong here? Is it something to do with file saving? (Is that necessary? Maybe it is a bit slow or something, or not seeing the new file?)

NealeTools commented 4 months ago

I also had a look at the current (last) index.bas file, assuming is still how this works. It has no line numbers in it and looks incomplete, is that normal?

image
NealeTools commented 4 months ago

When I get errors, I think some of the files are not being created correctly - perhaps not overwriting. As an example, iside the image I see (two BANK 22, and an empty BANK 21)

image
remy commented 4 months ago

Not gone through these yet, but do not apologise - you're literally helping by testing! These issues are really useful.

remy commented 4 months ago

There's a few things missing from context for me, but I'm not 100% sure how much difference it makes. I guess the first thing is:

  1. have you saved your .bas.txt file or is this an untitled file?
  2. This screenshot - is that index.bas or a different file? Only because the file being run is index.bas and I'm wondering if you might have saved a file and it's been sync'ed into cspect and you're inspecting that?

I'm going to through your example code to see if I can replicate too. Hopefully this evening, but it might be later on in the week if I don't find time.

NealeTools commented 4 months ago

There's a few things missing from context for me, but I'm not 100% sure how much difference it makes. I guess the first thing is:

  1. have you saved your .bas.txt file or is this an untitled file?
  2. This screenshot - is that index.bas or a different file? Only because the file being run is index.bas and I'm wondering if you might have saved a file and it's been sync'ed into cspect and you're inspecting that?

I'm going to through your example code to see if I can replicate too. Hopefully this evening, but it might be later on in the week if I don't find time.

That last screenshot was the code visible in the NExtBASIC editor after that last attempt. It seems that the index.bas being loaded was still the one prior to line 37 being added. The bugs seem to correlate with instances where I see two identically named BANK files in the devel directory. Interestingly, these duplicated BANK22 files cleaned themselves up after additional attempts (in case that gives any clues?)

If I browse the devel on macOS now, I see this: Three files, but note that BANK21 is listed as zero bytes...

image

Note that the current index.bas (now) looks OK: (as read by your online tool next to the encoded .txt file:

image

I just tried the most recent code again, but it again failed: This file is saved because VScode shows the X next to filename.

image

Notably, depsite failing, I broke into BASIC and launched a copy of the index.bas I made (the same one I showed convered in your online tool). It loaded up fine and ran fine.

image

Notably, in the .img remounting it shows four BANK files with BANK22 duplicated again. I suspect this is either killing the conversion or is a symptom of the failed conversion. But it is interesting that the BANK22 that NextBASIC chooses to load from its' own code is fine.

image

I'll inspect the difference between these BANK22 files next...

NealeTools commented 4 months ago

Both BANK22 are "identical" as viewed in macOS:

image

I then tried to open them in textedit. Remarkably, when I tried to do this one of the BANK22 files vanished!

image
NealeTools commented 4 months ago

ALl is weird...I now added some PAUSE 0 to test. Retried building. It worked this time! But this isn't the new code with PAUSE 0 in there! I assume it is the prior in index.bas that is loading up!

image image

I'm at a loss...

Even more testing: Now all the BANKs are behaving as if they have the same "MONKEY" code in them!...

image
NealeTools commented 4 months ago

OK. I did a quick test with a fresh .img—just to be sure it wasn't a problem with my extremely full devel folder. I emptied and then tried again. This time it failed again, and inspecing the .img shows:

image

Interestingly, trying to load TirstTest.bas gives Wrong File Type 0:2 error:

image

Yet, when dragged into your web tool, it looks good...

image
remy commented 4 months ago

The web tool is a couple of fixes behind, but also I don't think it knows how to handle bank splitting (partly because I was sure how I was going to do it yet!).

So that might actually be a parsing bug (in the txt2bas).

NealeTools commented 4 months ago

I wasn't trying to use the web tool to split the file. The index.bas is already tokenised. Given that it was apparently not loading in NextBASIC, I just wanted to see if your (older) webtool could convert it back to .txt and recognise the tokens (it can). I'm not sure if this provides more insight or not...

remy commented 4 months ago

Found it - or, at least I've made a fix that can't replicate, and the bug I fixed, I'm fairly sure is responsible for the confusion.

If you can read JavaScript, maybe you can spot it: https://github.com/remy/vscode-nextbasic/blob/master/lib/auto-run.task.js#L179-L197

Otherwise, basically, the main program wasn't being synced properly, yet the banks, kinda were.

There's a lot in your issue, so I'm hoping re-testing isn't too bad, and hopefully the fix does the job.

The latest vscode extension version is 1.11.2 which includes fixes to all three issues raised.

NealeTools commented 4 months ago

I can't read javascript very well... ;-) ChatGPT says: In the newer version of the code, there's an adjustment related to the file2bas function call, specifically the addition of the bankOutputDir argument. Let's take a closer look at this change:

In the newer version:

javascript Copy code writeFileSync( filename, file2bas(text, { validate: false, defines: true, bankOutputDir: tmpdir(), // <- New argument added here }), 'utf8' ); In the older version:

javascript Copy code writeFileSync( filename, file2bas(text, { validate: false, defines: true, }), 'utf8' ); In the newer version, the bankOutputDir argument is added to the options object passed to the file2bas function. This argument specifies the directory where the output files (banks) generated by the file2bas function should be placed. In this case, it's set to tmpdir(), which is a function from the Node.js os module that returns the operating system's default directory for temporary files.

By adding the bankOutputDir argument, the code instructs the file2bas function to output any generated files (banks) to the specified directory. This can be useful for organizing files and managing the output of the file2bas function, especially when dealing with multiple files or complex projects.

This change allows for more flexibility and control over where the output files are stored, potentially improving the organization and management of files within the application.

:-)

NealeTools commented 4 months ago

Great progress! Unfortunately, I still get an error (not a crash). It seems that contents of BANK 22 and BANK 20 in my test are identical:

image
NealeTools commented 4 months ago

Inspecting the .img shows BANK22 duplicated:

image

So I deleted them and tried again. First try failed with file not found. I tried again. This time it runs fine:

image

Can it be that there is some delay in the files being written into the .img?

NealeTools commented 4 months ago

Hmmm...I'd say that 9/10 times it crashes due to a malformed file that it can't load:

image

:-(

NealeTools commented 4 months ago

Sometimes even the index.bas won't load and it crashes to the autoexec.bas:

image
remy commented 4 months ago

So, replies:

Can I ask the following:

I can then inspect the raw byte data and hopefully also use it to replicate on my side.

remy commented 4 months ago

Wait, just saw your duplicated files, I've no idea how that can happen...it's literally a file system that should prevent that from happening... (that's confused me a lot... )

NealeTools commented 4 months ago

OK - I deleted devel. I just retried the test. It broke after loading up the index.bas:

image

I remounted .img and inspected. devel is back...but it is empty...how is that even possible, if CSpect loaded the index.bas? Is it loading from a cache?

image
remy commented 4 months ago

Is the image mounted whilst you're running cspect? It can't be, it causes weird file system effects. If it wasn't, I might be inclined to get a new image file and start afresh (I did this last week myself)

NealeTools commented 4 months ago

It is sometimes mounted, sometimes not. I've never had any problem with that. It doesn't update in macOS when it is mounted. So to see changes made by CSpect, I have to trash and remount. I've been working like this for years and not had any problems...

I just retired the test. It failed. So I went to browser to inspect...and this happened...

image
NealeTools commented 4 months ago

Here is the most recent attempt. One of the BANK files never got created, and one is zero bytes: devel.zip

remy commented 4 months ago

Cheers. I'll take a look at the zip in the morning.

I've had issues the whole time with mounting the .img file, often the system would eventually become read only and files wouldn't write properly, so I've trashed the .IMG a good number of times over the years.

I'll let you know if I find anything useful in there (I'm starting to suspect logs in the extension might be the way to debug though...)

NealeTools commented 4 months ago

OK - I just downloaded a new distro image, trashed the entire contents and replaced with the GitLab 2.08 verion. Ran my script and...

image

With new devel looking like this again:

image

BANK21 fails to build...

remy commented 4 months ago
SCR-20240308-iocw

Right. I'm there. I can replicate. That's the hard part down. Now to find out, what is actually happening!

remy commented 4 months ago

I'll add the auto incrementing lines is also hosed with split banks, as is the line number order validation - but I'll keep that as a separate issue.

The distilled issue here is (for future me)

I've checked $TMPDIR and the banks are there with correct sizes, but they're definitely not syncing properly.

remy commented 4 months ago

Sod, also just spotted this where:

no, the put calls are synchronous so cspect isn't launched until it's all across

However, I've just spotted the individual banks are actually using an async (whereas nothing else is). That's a bug, it could also explain what's going on. Just checking.

remy commented 4 months ago

SCR-20240308-itcf-min

Think I've got it.

remy commented 4 months ago

Right, 1.11.3 - give that a test, I think that's it!

(I'll file the syntax highlighting for bank split line numbers separately)

NealeTools commented 4 months ago

v1.11.2...A new crash I 'v never seen before lol!

image

v1.11.3: Oh yeah! :-)

image
NealeTools commented 4 months ago

Nice! And later references to the same FILENAME over-write, rather than crash. I think this is OK...unless you particularly want to try to catch this?

image
remy commented 4 months ago

I had that same bizarre stripy error too - I'm pretty sure it was loading random data from an "empty"/junk bank. First time I'd seen it before too!

NealeTools commented 4 months ago

This really is fantastic - thanks so much for taking the time developing this tool (and not to scare you off, but 2.09 is very soon to launch haha...).

Now...about the export feature: As I think I said before, I previously modified the .js file so that it additionally copied the resulting index.bas to a designated folder that I am syncing to the Next HW over wifi via .SYNC. I quite like that process because it means I have a single workflow that I can use for both emulator and HW in parallel should I choose to have both running. I guess that to do this now it will be a little more complicated (with all the BANKs that may be made), and I think I might need help.

One option: Could an additional export path be defined in the VSCode settings section of the Extension? Easily modifiable that way, and I'm sure you would also know how to correctly write the javascript so that the same files were copied to both the .img and to this additional path.

remy commented 4 months ago

I'm not sure what your workflow is, but I've been doing this myself for the games I wrote using the export functionality.

I also sneaked in key binding (cmd+shift+e) will bring up the dialog box, though it default to the working directory, you can point it to any directory and it'll export all the files to that location.

NealeTools commented 4 months ago

Hi Remy - I've just tried this out, but I suspect there may be a small bug remaining that I didn't notice before. The export function created what appear to the the correct files in the working directory (GeneralCode) - top window. FirstTest.bas is actually the master .txt file so ignore that (I confusing name these .bas but they are .txt files!) Untitled.bas is the tokenised main .bas prog (because I didn't give it a name during save), and the BANKs are the split off files. Note, however, that the BANK files are recognised as .txt files by macOS (this is useful...but unusual).

In the lower window, I did a RunInCSpect version—and what you can see is a view in the remounted devel folder. As you can see the BANK files are now listed as Unix executable files. This is what I normally see for banks. I'm not sure if this difference matters, or why there would be a difference if the files are identical. Secondly, for unknown reasons, "Untitled.bas" is also present in the devel folder. Where did it come from? I'd cleared out the devel folder to be empty—and remounted to confirm it was empty—before running the RunInCSpect macro.

image

EDIT: Upon inspection in TextEdit, the two tokenised versions of the BANK files seem (visually) identical. However, the tokenised FirstTest.bas and Unitled.bas are subtly different. (Note: the two Untitled.bas, one in GeneralCode, one in devel, appear identical.)

image
remy commented 4 months ago

So, I believe all this is actually fine.

  1. Your mac picking up that the BANK1 is a text file is entirely based on your finder settings. I mean, it's wrong to think it's a text file, but the mac isn't actually looking at the file's contents, and only the file association.
  2. Everything in the working directory is synchronised across to the the /devel directory when you test with cspect - so if you exported anything into the working directory (including sub-directories), it'll go across to cspect
  3. For my own personal dev, I keep my plain text (source) files clearly labelled as text (where you're naming them as .bas - which is actually a binary format). For example, I would have FirstTest.bas.txt (so I know that this is source and not something I can drop into a spectrum, but also can open with any text editor). I also tend to organise the directory so that there's two sub-directores: src and dist - src containing the source text files, and dist contains the generated binary .bas (and banks). But, here's really what you prefer.

Finally as for the byte differences between Untitled.bas and your index.bas - if you zip them up I can tell you what they are. Though you can drop the index.bas into VS Code, it'll say "The file is not displayed in the text editor because it is either binary or uses an unsupported text encoding." - but open it anyway, then in VS Code's command palette, select "NextBASIC: import binary .bas to text" and it'll show you the bas2txt source.

NealeTools commented 4 months ago

Thanks Remy!

  1. Ah - yes, but any idea why it detects some of the BANK files as unix executables (the ones generated in the .img) and some as if they are text (the ones generated by the export)?
  2. Of course!
  3. Yes - the reason I named them .bas was a workflow fudge so that I could specifically associate by BAS text files with VSCode (for auto-opening), but not for all .txt files (which I prefer to open in TextEdit). It's not great...and probably I should make a bespoke suffix. I may see if .bas.txt be associated globally with VSCode in macOS.

As for index.bas vs untitled.bas: Quite a few byte differences...

image

And after bas2txt:

image

Aha! It looks like autoline numebring is enabled in the Export ("Untitled") option! This explains the byte differences. :-)

Also, the "Untitled.bas" is not saved with an autostart option. I can load it in NextBASIC and it opens to the line listing:

image
remy commented 4 months ago

Of course. Yes, index.bas has autostart because it needs to launch when developing, but if you want that in the file, you actually need to define it (same way as you would with .txt2bas)

NealeTools commented 4 months ago

Ah - OK, so I just add: "#autostart 1" at the beginning of the code. However, how would I turn off renumbering of the exported version? That seems to be being applied erroneously (unless there is a global export option somewhere I've missed?) Thanks!

remy commented 4 months ago

Renumbering is opt-in in your code. Are you seeing it happening and changing your source numbers? If that's the case, can we raise a new issue with example code?

remy commented 4 months ago

Ah, I'm only seeing the screenshots now (I get the messages in email, but not the images).

Let's open a new issue with the autoline thing.

(just a note, if you .bas file contains autostart, it'll add those lines it in when importing from bas to text)

NealeTools commented 4 months ago

I raised a new issue #51