Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BDT parsing not working properly with files in root path #2

Closed
kounch opened this issue Jul 8, 2020 · 7 comments
Closed

BDT parsing not working properly with files in root path #2

kounch opened this issue Jul 8, 2020 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@kounch
Copy link
Owner

kounch commented Jul 8, 2020

As reported on Facebook:

If the BDT generator is pointed to /TEST/SUBFOLDER the resulting BDT file looks like this (partial):

/TEST
Program in TEST,1,,Program in TEST.tap
Program in SUBFOLDER,1,SUBFOLDER,Program in SUBFOLDER.tap

On the Next, this launches both programs perfectly.

If it's pointed to the root instead, the BDT file looks like this:

/
Program in ROOT,1,.,Program in ROOT.tap
Program in TEST,1,TEST,Program in TEST.tap
Program in SUBFOLDER,1,TEST/SUBFOLDER,Program in SUBFOLDER.tap

In this case all programs give a File Not Found error on the Next.

If I manually edit the BDT file before generating the cache and I change the ',.,' to ',,' to match the format of the first example, the first program will load but the other two will still give File Not Found errors.

If instead I change '/' to 'C:/' this will load the second and third programs, but the first will give a File Not Found.

If I make both changes, all of the programs give a File Not Found again. Maddening.

It looks as though in the subfolder version there's no trailing '/' in the home path or the folder paths, but there are implied '/'s between the home path and the subfolders and between the subfolders and the files. If this implied '/' is also being applied in the root version, I'm wondering if this is what's generating invalid paths?

@kounch kounch self-assigned this Jul 8, 2020
@kounch kounch added the bug Something isn't working label Jul 8, 2020
@kounch
Copy link
Owner Author

kounch commented Jul 8, 2020

It looks like a parsing problem no the loader side, rather than the BDT creator.

This version of the loader tries to sanitise better the directory path creation and may fix it.

knloader.bas.zip

kounch added a commit that referenced this issue Jul 9, 2020
@KevReillyUK
Copy link

KevReillyUK commented Jul 9, 2020

I've had a look at this and there seem to be two related issues when the BDT file and cache are pointed at the root folder.

The first is that KNLOADER.BAS can't actually confirm the existence of files in the root of the drive, and so gives a Not Found error. I'm guessing this a path construction error of some kind but I didn't dig too deeply.

The second problem is that the temporary path information file/array generated by KNLOADER.BAS stores it like this...

/<PATH>
<SUBFOLDER>
<FILENAME>

...and KNLAUNCHER adds / characters between the three elements when in reads it back. This is fine for non-root structures where this...

/PATH
SUBFOLER
FILENAME.EXT

...becomes this...

/PATH/SUBFOLDER/FILENAME.EXT

But for root-based structures...

/
SUBFOLDER
FILENAME.EXT

...becomes...

//SUBFOLDER/FILENAME.EXT

...which is an invalid path because of the extra /

I'm not sure there's a single fix for both of these. The second one seems easier in principle (if the root is / on its own, treat it as null and let KNLAUNCHER put it back) but in practice I couldn't get it to work. Your self-erasing code, and me forgetting to save my own edits, defeated me one too many times. I need more practice with the Next and its quirks!

Looking at the way the various programs pass data to each other, it may be easier to trap these errors in KNLAUNCHER rather than KNLOADER.BAS.

kounch added a commit that referenced this issue Jul 9, 2020
@kounch
Copy link
Owner Author

kounch commented Jul 9, 2020

You are right, the previous fix only looked at knloader.bas and not to knlauncher

The attached file should fix it on knlauncher side.

knlauncher.zip

However, i don't have time today. I will try to test it tomorrow, if i can find some time.

Both changes are also committed on the develop branch, here:

https://github.com/kounch/knloader/commits/develop

@KevReillyUK
Copy link

Thanks. That fix worked for files in subfolders and sub-subfolders of root, but not those in the root itself.

The two lines below if added to KNLOADER.BAS will fix the root files problem, but there may be more efficient ways of achieving this in KNLAUNCHER or the BDT/cache builders. It's getting a bit late for experiments but as far as I can tell there's still something funny going on with the building of paths to the "home" directory, which are null when "home" is a folder but are '.' when "home" is root. I think this may be what the second line here is working around.

Anyway, hopefully these will give a clue as to a more permanent solution:

975 IF c$="/." THEN c$="/" :; Prevent file checker from failing with a Not Found when checking root

1015 IF a$="." THEN a$="" :; Prevent invalid path giving ERROR 45 in KNLAUNCHER

@kounch
Copy link
Owner Author

kounch commented Jul 11, 2020

You are right. The "." at the end of the path is breaking it.
Latest commit should fix it. At least in my testing environment is working fine.

Could you try to download the attached zip file (with knlauncher and knloader.bas inside) and see if they fix it also for you?

Thanks for your help!

3d936589.zip

@KevReillyUK
Copy link

Yes, that seems to have solved it for both root-based and folder-based start folders. I started over with a fresh install of Beta 11 and then added the two new files over the top, just in case I ended up with another broken hybrid.

(I did spend about ten minutes wondering why it wasn't picking up two programs in a subfolder of my DEMOS folder, before realising they're both .TRD files and so can't be loaded natively anyway! The worst bugs to find are the ones that only exist in your own head.)

Thanks again for persevering with this. The likelihood of me using root-based structures with this program "in the wild" is very low, but it's nice to know it'll work if I do.

@kounch
Copy link
Owner Author

kounch commented Jul 11, 2020

Great!

Released 0.12 Beta including this fix.

@kounch kounch closed this as completed Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants