Add y-offset to overlays.

[Lknadfodr]

Draft for Issue 1966
The game runs and the sprites are correct.

However I have no knowledge about the links array and merging them.

[Flamefire]

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?

[Lknadfodr]

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.
In JOBS.BOB there are 2269 overlays and 8928 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:
In CARRIER.BOB there are 1011 distinct combinations, meaning 2253 duplicates use the same link+ny.
In JOBS.BOB there are 2464 distinct combinations, 6464 are duplicates.

I'll update so it will load each overlay just once.

In general I see two options:

1. We apply the ny-correction right away. In this case, the archive has the same overlay image multiple times with different ny values. This keeps the change opaque for the rest of the code base.
2. We do not apply the ny-correction here, but instead offer a new method like getOverlayNyCorrection(overlayIdx, fat, direction, animationstep). Then the caller has to retrieve that seperately and apply it when the full sprite with body+overlay is assembled.

Let me know if you prefer option 2.

[Flamefire]

Ok so we have multiple dimensions. Let me try to unpack:

• std::vector<uint16_t> links; /// Array [overlayId][animStep=8][fat=2][direction=6] mapping to an overlay picture
  This is a mapping of the overlay to an image index
• Each image might be used for multiple overlays but the y-offset might be different

It makes sense to

1. Load each image exactly once -> reduce loading time
2. Store each image exactly once -> reduced memory footprint

I'd say we use the original approach similar to option 2:

• Load each overlay once -> keep links
• Load y-offsets
• getOverlay returns a pair of image and y-offset as you always need both, don't you?
• Rename "links" to e.g. "imageIndex" and update documentation/docstrings to make it easier to understand. Pick something you'd have liked to be able to understand the mechanism. Like "mapping of each overlay (index) to actual bitmap (index) stored in memory"
• Similar for a global y-offset/ny array to be added

[Flamefire]

Looking good. Might need formatting (clang-format 10) at the end, see CI

[Flamefire]

I was concerned about the writeLinks method. I expect there is some other code or tool that uses that file, so I didn't want to change the values .

Good catch! I found: https://github.com/Return-To-The-Roots/s25client/blob/f51945885ec12f82c0766c25d38e3177efae7461/libs/s25main/ogl/glArchivItem_Bob.cpp#L23-L35

This is called with the links as read from the file, i.e. the shifted ones. Of course one could talk about whether this is "correct", but I'd say yes:
It is intended for overrides, usually: Folders with image files. And when you unpack a BOB file you get the overlays at the shifted positions. Unfortunately we miss any mechanism for the ny offsets there

[coveralls]

coverage: 83.372% (+0.02%) from 83.356% — Lknadfodr:issue-1966 into Return-To-The-Roots:master