-
Notifications
You must be signed in to change notification settings - Fork 73
Jsatom #505
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
base: main
Are you sure you want to change the base?
Jsatom #505
Conversation
… are marked with // ATOM Added ROMS and MMC. Added 6847 with font data. Added PPIA and MMC. Added new menus and TV set. Audio and Tape code added but not tested. Will do more tidy up.
added PPIA to the DEBUGGER for Atom
…nce it can be Gb.
Still needs testing since wildcards don't work. I'm not sure how it is intended to be used. made an empty MMC (with a readme)
|
Wow fantastic! I'll try to get to this in the next few days. Thank you so much!! |
mattgodbolt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some very early first comments. This is a huge amount of work! Thank you!!!
What I think is best is if we go back and forth a little on this to "level set" on the changes.
Then, as we get nearer, I'll look at merging some file parts that can be done already.
For example, we could merge the sound chip changes pretty much as they are, and so on. That way we will gradually make small changes that add functionality to the emulator, and this PR will shrink down as its capabilities merge in, then finally we'll merge this to "turn it all on". etc :)
Thoughts? :)
| package-lock.json | ||
| *.png | ||
| *.zip | ||
| .prettierignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't put the file in itself here. prettier can format its own configuration file just fine :)
Why did you have to ignore png and zip here? prettier shouldn't have tried to format anything it didn't understand, and that includes binary files.
Can you recall why you added these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During commit, the pre-commit process was stopping me commit. I was probably tired and overzealous.
README.md
Outdated
| file assumed to be within. | ||
| - (mostly internal use) `logFdcCommands`, `logFdcStateChanges` - turn on logging in the disc controller. | ||
| - `audioDebug=true` turns on some audio debug graphs. | ||
| - `model=atom` jumps into the hidden [Acorn Atom emulator](README-jsatom.md) with preinserted AtomMMC zip file. Use `model=atom-tape` for just the Acorn Atom without MMC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice :) We can consider making this a proper "model" in a follow up PR if you'd like.
| <a href="#google-drive" class="dropdown-item" id="open-drive-link">From Google Drive</a> | ||
| </li> | ||
| <hr /> | ||
| <li class="dropdown-divider"></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? Not against it, just trying to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It felt more 'bootstrap'y
index.html
Outdated
| </li> | ||
|
|
||
| <!-- | ||
| <li class="nav-item dropdown embed-hide"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not add commented in code unless we really need to, this file is already big enough :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that. I have a nasty habit of leaving in work-in-progress. I need a new habit to remember to clear it out.
| content="script-src 'self' 'unsafe-inline' 'unsafe-eval' *.google-analytics.com *.google.com;" | ||
| /> | ||
| <title>jsbeeb - Javascript BBC Micro emulator</title> | ||
| <title>jsbeeb - Javascript BBC Micro emulator (incl. Acorn Atom)</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: The changes in this file are pretty huge, and hard to follow. That's on me for not making this more modular -- now I'm building things with vite we can probably make this more modular. But for now we might be OK. I'm certainly not expecting you to make that change :)
| this.updatePrevMem(); | ||
| this.cpu.execute(1); | ||
| self.debug(this.cpu.pc); | ||
| this.debug(this.cpu.pc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good spot!
| */ | ||
| export function parseMediaParams(parsedQuery) { | ||
| const { disc, disc1, disc2, tape } = parsedQuery; | ||
| // ATOM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's now broken for BBC? You've commented out the BBC one and returrned only the ATOM one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry
| if (hostname.startsWith("bbc")) return "B-DFS1.2"; | ||
| if (hostname.startsWith("master")) return "Master"; | ||
| // ATOM | ||
| if (hostname.startsWith("atom")) return "Atom"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice - this will mean atom.xania.org would return an Atom. I can set that up elsewhere.
src/tapes.js
Outdated
| this.cpuSpeed = 1 * 1000 * 1000; | ||
| } | ||
|
|
||
| _isAtom = this.isAtom; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this local variable that we then check only on the next line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use it later in that function but it could be replaced with this.isAtom instead.
|
Thanks! To set expectations, I'm around this week then on holiday for 2w with limited internet so ... this might take a while! I appreciate your patience! |
|
(I'll make some plans though to, for example, bring in one zip/unzip library into the mainline and then see if I can refactor your code to use it? things like that. Might be a long slow burn this one) |
|
I dug about a bit and |
|
OK I just hit the |
| this.lastCtrlLocation = 1; | ||
| this.lastAltLocation = 1; | ||
|
|
||
| // set up for BBC or Atom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the atom have both a VIA and a PIA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No VIA, just the PIA
|
Happy with the slow burn. It took me months to get it working, then roughly 5 years of minor tweaks when I had the time. |
|
thanks @CommanderCoder - I just got back from a two week holiday so ... I'll get there! |
pointing to commandercoder version to get AcornAtom
…t out of the generator.. It missed some devices and had empty catch blocks, so used Copilot to generate those too.
…reState functions
now able to put 'restore' as a query string and pass a URL to a state file (json) Modernised some of the state code
… loading PCHARME and GAGS into first two BANKS as ROMs
I've been working on the JSAtom version of JSBeeb since 2020. Originally it was separate from JSBeeb but I've merged it with JSBeeb now. To get access, look at the 'About JSBeeb' page and it has the hidden JSAtom link.
It all works (what I've tested anyway). It can load from MMC (defaults to load Hoglets MMC), create a blank MMC, download MMCs. It can load off discs (using -DOS version) or of cassette tape (using -TAPE) version. I've not testing saving to tape or disc since MMC works fine. Sound works, and colour, and it manages to do mid-frame mode cahnges on the 6847 video chip.
I had to import JSZip ( https://stuk.github.io/jszip/) for the MMC zip and unzip.
Keyboard matches the Acorn Atom as closely as possible, but that means CTRL is used for shift key. CTRL-F12 will issue SHIFT-BREAK and boot *MENU. Left and Right with < , >. Type a letter to boot the program.
No SID support