Draft: add y-offset to overlays, store complete pictures. - #29
Conversation
Flamefire
left a comment
There was a problem hiding this comment.
Just thinking out loud as I don't really know the link-stuff either. So please correct me.
I think the biggest change is:
- set(NUM_BODY_IMAGES + links[i], std::move(image));
+ set(getOverlayIdx(i), std::move(image));i.e.
- set(NUM_BODY_IMAGES + links[i], std::move(image));
+ set(NUM_BODY_IMAGES + i, std::move(image));previously get used archive[NUM_BODY_IMAGES + links[idx]], now we have archive[NUM_BODY_IMAGES + idx]
So that does look the same
With the removed if(loaded[links[i]]) continue I think the above is correct but stores duplicates.
I.e. it looks like bob-idx x and y could previously use the same image z if their links[x] == links[y] == z
Now we store that twice: At x and y instead of only at z but that allows us to set ny directly
I guess that is negligible as we unwrap it during loading.
What we could do is a) remove the links member and b) still keep it local to the method so that when we have already loaded an image we just clone it and adjust the ny instead of loading it from scratch.
Can you check how often this "sharing" actually happens to see if this is worth it?
|
I checked the number of overlays and the number of complete pictures, here are the results: In CARRIER.BOB there are 602 overlays and 3264 complete pictures. So, loading each overlay just once actually saves quite a lot (~80% for CARRIER, ~75% for JOBS). While at it, I also counted how many different combinations of overlay-link and ny-correction there are: I'll update so it will load each overlay just once. In general I see two options:
Let me know if you prefer option 2. |
|
Ok so we have multiple dimensions. Let me try to unpack:
It makes sense to
I'd say we use the original approach similar to option 2:
|
| uint16_t getOverlayIdx(unsigned idx) const; | ||
| uint16_t getOverlayIdx(unsigned overlayIdx, bool fat, ImgDir direction, unsigned animationstep) const |
There was a problem hiding this comment.
I'd say we can inline this into getOverlay
| return ErrorCode::UNEXPECTED_EOF; | ||
|
|
||
| if(links[i] >= numOverlayImgs) | ||
| if(link >= numOverlayImgs) |
There was a problem hiding this comment.
I do like having a local variable link even when we keep the links[i] as it makes it easier to read below to not have links[i] all over
Draft for Issue 1966
The game runs and the sprites are correct.
However I have no knowledge about the links array and merging them.