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

Copying a BASIC program alerts constantly with "problematic byte 13" #107

Closed
mattgodbolt opened this issue Nov 2, 2024 · 2 comments · Fixed by #121
Closed

Copying a BASIC program alerts constantly with "problematic byte 13" #107

mattgodbolt opened this issue Nov 2, 2024 · 2 comments · Fixed by #121
Assignees

Comments

@mattgodbolt
Copy link
Owner

I fixed an error where this.emulator.readmem wasn't defined. But...looks like there's code that alerts all the time:

                while (len--) {
                    let b = this.emulator.readmem(addr++);
                    if (b < 32 || (b >= 127 && b < 161)) {
                        if (b == 10 || b == 13) alert("problematic byte value " + b);
                        b += 0x100;
                    }
                    data += String.fromCodePoint(b);
                }

@8bitkick I'm not sure what this is supposed to do but copying any memory pretty much kills the browser with this (as it has a modal alert up)

@ojwb
Copy link
Collaborator

ojwb commented Nov 5, 2024

Sadly @8bitkick broke my saving memory feature - I pointed this out in #96 nearly 2 years ago but I never got a response...

@ojwb
Copy link
Collaborator

ojwb commented Jan 21, 2025

BTW this feature is intended for uses like putting assembler code into a comment in the program - e.g. you can use inline assembler to assemble the program as if it's at PAGE+5 but output it at &A00:

O%=&A00:P%=PA.+5:F.O=4TO7S.3:[OPTO
; some assembler
]N.:P.O%-&A00

Then you can copy the reported number of bytes from &A00 and paste them into the editor after a REM as the first line.

Or add REMxxxxxxxxxxxx... as the first line and then just assemble direct to PA.+5 even.

Another use is to create pre-computed binary data using BASIC to stick in a REM or similar.

Trying to copy the tokenised BASIC program code isn't the intended use (also not a very useful non-intended use AFAICS). The default address and length being the BASIC program was not a good choice. Not sure there's really a good useful default but we could default to an interesting string from the ROMs. The current value of O% could be a reasonable default for the end address, but the start address is harder without trying to parse the program to find what O% gets explicitly set to, or track when the emulator sets O% as it executes (O% lives at a fixed location in RAM so easy to read - in BASIC code it's: !&43C).

The reason for the alert is that bytes 10 and 13 are problematic in program code sent to the bot, even if encoded using Unicode (e.g. as U+010A/U+010D) - it may require avoiding certain assembler instructions, constant values or branch offsets (or data values for the data case). ISTR 13 (encoded or not) is inherently problematic (because BASIC sometimes skips lines based on looking for it) but U+010A (and other Unicode encodings of 10) could probably be made to work with some tweaks to the bot code.

The number of alerts should be finite (and are in my testing) but multiple duplicate alerts are unhelpful and it'd be better to count how many of each we saw and report a single alert if we saw any summarising how many of each.

I can try to improve this, and I guess now you've fixed this.emulator.readmem not being defined the UI change issue and memory copying issues aren't really entwined issues.

However I'd still really like to resolve the changed placement of controls - they still feel illogical to me now - the "save memory" takes memory from the emulator not from the editor so logically should be with the emulator pane - if you run, edit the program, save memory what you save doesn't reflect what's now in the editor which even tripped me up just now checking the alert behaviour.

The icons also seem much less clear than the words were (and I miss the cute detail that the buttons were BASIC keywords).

If active use of the editor on mobile is a real thing (and those log entries are not just people clicking on the link from the bot post and only viewing the code) then we could presumably make the buttons icons for mobile but words for desktop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants