Bulk multibyte

[SuuperW]

To enable bulk poke/peek access for 16 and 32 bit values, I have given MemoryDomain , IMemoryApi, and MemoryLuaLibrary some new functions.

Check if completed:

• I have run any relevant test suites
• I, the commit author, have read the licensing terms for contributors (last updated 2024-06-22) and am compliant

[YoshiRulz]

On top of cleaning up IMemoryApi, I would also like to clean up Range<T> (#3965) before adding new uses to MemoryDomain.

[YoshiRulz]

Write*Range signatures are fine for Span, but then why do Read*Range allocate and return arrays?
See also #3472.
Not that you need these anyway, since the caller can just MemoryMarshal.Cast to/from byte.

[SuuperW]

I think it was just a lot simpler to do it that way, and the byte one returns an array IIRC so I was following that.
I knew you were wanting to update some of this stuff anyway, so I didn't think it very important.

[SuuperW]

Update on this: The new read methods return IReadOnlyList, so as to match the existing ReadByteRange method. I considered changing them all to accept a Span instead but that's too much work to do before release.
The existing WriteByteRange method was changed to accept a Span though, since this is good and easy to do and makes it match the new ones.

[YoshiRulz]

Do not change IReadOnlyList<byte> to Span<byte> in WriteByteRange's signature.

The added methods are redundant, but if you're going to add them, don't use Span there either.

[SuuperW]

Do not change IReadOnlyList to Span in WriteByteRange's signature.

Why not? Because it changes the API for external tools?

The added methods are redundant

PLEASE, quit saying this. I have already explained to you that writing two adjacent 8-bit integers is not always equivalent to writing a 16-bit integer.

don't use Span there either.

Why? This isn't even an API change, and Span can be more efficient.

[YoshiRulz]

The added methods are redundant

PLEASE, quit saying this. I have already explained to you that writing two adjacent 8-bit integers is not always equivalent to writing a 16-bit integer.

I thought your comment below was in reference to cores' implementations.
Poking two adjacent 8-bit integers in virtual/logical memory is always equivalent to poking one 16-bit integer (assuming the endianness matches). If it's not, that can't be considered poking, and the core needs to be fixed.

Do not change IReadOnlyList to Span in WriteByteRange's signature.

Why not? Because it changes the API for external tools?

don't use Span there either.

Why? This isn't even an API change, and Span can be more efficient.

There's a reason we're not already using Span everywhere, and that's because it's not always obvious what design and signatures should be used. Which is because there are many related types, some struct and some class, each with their own quirks and limitations. I don't pretend to be an expert in them, and clearly neither are you since you used the mutable Span here with no intention of writing to it.
As I have said before, making repeated changes to the interfaces that people use (in this case an API) is what you might call "churn" and should be avoided. Accepting a Span as input only to change it to e.g. ReadOnlyMemory later is annoying. Accepting a Span as input only to do a bunch of copying anyway is deceptive.

[SuuperW]

I thought your comment below was in reference to cores' implementations.

And how would a core who's implementation differentiates between two adjacent 8-bit writes and one 16-bit write be able to make that distinction for bulk operations if the only bulk operation hands everything over as bytes? Or would you want the MemoryApi methods to call PokeUshort in a loop? That would prevent the core from optimizing bulk operations.

virtual/logical memory

Not all memory addresses refer to virtual or logical memory. melonDS has addresses which map to registers, and even has some that behave more like streams than traditional RAM. (Writing 1 then 0 is not the same as writing just 0, and reading a value can cause things to happen including causing the next read to be different.) Saying "change/poke the value at this address" is meaningless because there is no value at that address. The only thing you can do is write to that address or read from it. Unless you create a new API for handling these things, the existing memory API must support it.

There's a reason we're not already using Span everywhere, and that's because it's not always obvious what design and signatures should be used.

And all of this somehow does not apply to ReadOnlyList? There is is somehow obvious?

Accepting a Span as input only to do a bunch of copying anyway is deceptive.

I disagree. There's nothing indicating an intent to write to the span. But changing it to a ReadOnlySpan is a good idea anyway.

As I have said #4012 (comment), making repeated changes to the interfaces that people use (in this case an API) is what you might call "churn" and should be avoided.

The comment you linked does not mention "churn" and I don't see how changing keybinds is related to this. I do agree changes should be kept to a minimum, but that doesn't mean we should not use a span for the NEW methods.

[YoshiRulz]

Poking two adjacent 8-bit integers in virtual/logical memory is always equivalent to poking one 16-bit integer (assuming the endianness matches). If it's not, that can't be considered poking, and the core needs to be fixed.

Not all memory addresses refer to virtual or logical memory. melonDS has addresses which map to registers, and even has some that behave more like streams than traditional RAM. (Writing 1 then 0 is not the same as writing just 0, and reading a value can cause things to happen including causing the next read to be different.) Saying "change/poke the value at this address" is meaningless because there is no value at that address. The only thing you can do is write to that address or read from it. Unless you create a new API for handling these things, the existing memory API must support it.

What you're describing with register-like addresses on the system bus is clearly not poking. The domain(s) with that behaviour should be Writable: false, or maybe throw an exception with those specific addresses (not sure if there's precedence for that).

[SuuperW]

Regarding all of your comments about being able to just convert between bulk byte and bulk int reads: No. The only reason I made this is to support #4555. The core MUST know what size we are reading, and the various sized reads cannot simply be re-interpreted as another.

[vadosnaprimer]

Conflicts appeared, but also it's unclear from the discussion if there's anything else that has to change still.

[SuuperW]

I was going to wait for Yoshi to finish #3965 but I can go ahead and fix some stuff here in a few days.

[vadosnaprimer]

I was going to wait for Yoshi to finish #3965

That one looks like it will require 20 more years to get done abandoned forever.

[SuuperW]

Fixed bugs and added tests for the bulk API methods. That code is absolutely horrible, matching the existing bulk byte methods.

IMemoryApi.WriteByteRange was changed from accepting a IReadOnlyList<byte> to Span<byte> which is technically an API change but byte[] works for both without an explicit cast. That lets it use the new underlying MemoryDomain.BulkPokeByte, and it also what the new bulk poke API methods do. If we need the version that accepts a list, I can make overloads for them.