feat(world,store): add updateInField

[dk1a]

Like pushToField, but only updates existing values. Same as push this does not yet introduce any new events and won't break networking.
I've considered merging them into some kind of splice, but that'd be very clunky and inefficient in solidity; a single splice event might make sense though.

A questionable decision here is to silently ignore non-existent indexes (technically they revert because of fullData, but that's only for event compatibility), that's because with an update/splice event I wouldn't need to ever read the length, so not checking against it saves a storage read

[holic]

Is this blocking anything for v1 parity prior to hackathon? Otherwise, it might make sense to hold off on this until next week.

[dk1a]

Not blocking or critical, but without it dealing with dynamic tables gets extremely annoying as you have to get the full data, change an item, then set the full data

[holic]

is this the field index or byte index?

[dk1a]

byte index. I've considered startByteIndex but that seems long, and is abstracted by tablegen anyways. wdyt?

[holic]

I generally always prefer longer, clearer variables where it's otherwise hard to glean from context. I think it would make sense here.

[holic]

another option is just byteIndex (to match schemaIndex) or byteOffset (Uint8Array constructor and our decoding code calls it byteOffset, fwiw)

[holic]

is there a better name for this? spliceField or setFieldSlice or something?

[dk1a]

I tried to make splice actually, but the proper startIndex->delete->push can't work well in solidity, and update-only splice looks kinda misleading.
setFieldSlice isn't bad but I'd also want to consider its tablegen pair as well. So we have get, set, push, and:
update / slice / setSlice / setItem ...
updateCol1 / sliceCol1 / setSliceCol1 / setItemCol1 ...?

[dk1a]

added this to v2 tracker so we can come back to it

[alvrs]

lgtm! I'd be curious to see how this additional method in StoreCore and World affects the World contract size. Could you add a quick benchmark for that here for reference? (Since we're already really tight on the limit in the World contract due to all the inlined libraries)

[dk1a]

World 20.726 -> 22.201 kB
I suppose we have enough space for maybe 1-2 more methods

[alvrs]

22.201

Oh damn that's getting really close - i've experienced the bytecode being slightly larger in consuming projects (eg v2sandbox) for some reason, 22 might already push it over the limit there. We should make finding a way to "tree shake" the included libraries a priority (eg by using free functions instead of libs?)

[dk1a]

We should make finding a way to "tree shake" the included libraries a priority (eg by using free functions instead of libs?)

After some brief testing I think solidity already does this, using free functions doesn't seem to change anything (I could've missed something, tried it only on a couple libs)

[dk1a]

I could separate the array stuff (push, update, pop) from StoreCore (StoreDynamic?) and World (install another system in CoreModule?), then it should go back to ~20

[alvrs]

I could separate the array stuff (push, update, pop) from StoreCore (StoreDynamic?) and World (install another system in CoreModule?), then it should go back to ~20

Happy to merge this as is and find a way to reduce World's byte code in a follow up (high prio though).

Generally I think the Store methods should be part of World's bytecode, because they're a hot path for system's internal logic (we probably wouldn't want to add overhead for forwarding to another system to handle store methods). Would be interesting to measure how big StoreCore itself is right now.

[dk1a]

Generally I think the Store methods should be part of World's bytecode, because they're a hot path for system's internal logic

if intra-array methods are separated at the level of Store, then the intra-array system inherits StoreDynamic and should be entirely self-contained, and then gets registered on World