z00m128 / sjasmplus

Command-line cross-compiler of assembly language for Z80 CPU.
http://z00m128.github.io/sjasmplus/
BSD 3-Clause "New" or "Revised" License
383 stars 54 forks source link

Adds AMSTRADCPCPLUS and SAVECPR #217

Closed lordheavy closed 8 months ago

lordheavy commented 1 year ago

I updated documentation and kate syntax highlighting. Tests are missing - help needed !

All current tests passed.

redbug26 commented 1 year ago

Great job for Amstrad owners !

ped7g commented 1 year ago

I had a quick initial look and this looks very good, thank you.

One question I have is what is the exact relation between 464, 6218 and plus. The way you wrote it they are completely separated, so savecpr is available only for plus, and savecdt, savecpcsna are available only for 464 and 6128. Are the machines that different and there's no overlap, that neither CPR or CDT is useful in the other model?

From a quick look the Plus seems to be same as 6128, only having more RAM, so it's not clear why for example CDT wouldn't be beneficial also for Plus machine. And other way, the .cpr don't support the smaller ones? (Pardon my ignorance, but I'm not familiar with CPC machines at all).

If the directives could be useful in cross-way, I would suggest to add the Plus into the current cpc family (IsAmstradCPCDevice), and add extra logic in savecpcsna/savecdt to account for third machine (no idea if snapshot make sense). And similarly if savecpr makes at least some sense from 464 or 6128, making the savecpr support them too, so whichever CPC device is user using, all meaningful directives will be active and supported.

lordheavy commented 1 year ago

Thanks for the quick review.

The amstrad plus machines are:

To keep code simple, I chose to focus the amstradcpcplus device to support the cartridge through the cpr file format. The 32 pages available for this device are more an image of the cartridge than an image of the ram (extra 64k of ram for the 6128(+) is mostly useless when you use a cartridge).

In a first patch, I added sna and cdt feature, then I changed my mind, because some more device should be added to manage ram differences between 464+, GX4000, 6128+, and hardware support differences (GX4000 only support cartridge). But this could be introduced in a future patch.

lordheavy commented 1 year ago

Anything new ?

ped7g commented 1 year ago

Sorry, I had lately to minimize the time for hobby coding stuff. I do have this pretty high on the list, and I'm hoping to get back to it still this year, but can't give any promises right now. But I don't want it to fizzle out, should be part of next release, whenever that happens.

ped7g commented 8 months ago

merged manually on master branch, TY lordheavy.