Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R16 support for Dune 2000 #21170

Closed
wants to merge 1 commit into from
Closed

R16 support for Dune 2000 #21170

wants to merge 1 commit into from

Conversation

IceReaper
Copy link
Contributor

While the Tileset R16 files include 16bpp sprites only, Data.R16 and Mouse.R16 include 8bpp files too. So viewing them in the asset browser still requires the right palette to be selected.

Copy link
Contributor

@anvilvapre anvilvapre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal to change the name to Rx or Raster.

File format check is not very robust. It couldn't by conincidence now conflict with the other format i..e shp?

Type byte field that is read from the stream. I would make a byte enum for this to indicate the 1 = 8bpp and also check if otherwise 2?==24rgb? I.e. in all cases where it currently is not 1 you do not expect a pallette. So you use both type and bpp to determine the format.

Perhaps add support to to check if it is either a 8bbp or 24 rgb type.

Constructor of frame searches the first non zero byte. Doesn't seem very robust.

{
var color16 = s.ReadUInt16();

Data[i++] = (byte)(((color16 >> 7) & 0xf8) | ((color16 >> 12) & 0x07));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional. Add some comment explaining the magic. May help future developers.

0xf8=11111000.

File format is bgr and you store rgb?

@penev92
Copy link
Member

penev92 commented Nov 5, 2023

Can we go with D2kSprite(Loader) as a name instead?

@IceReaper
Copy link
Contributor Author

Tbh I heavily dislike file format detection anyway. The more formats you have, the more performance is wasted. It even sometimes misses a format. It's okay imo for formats with a header signature. But all those "try if you can parse it"-stuff is pretty bad. IMO it might make more sense to map extensions to loaders in mod.yaml. or better a path they to a format. But that's not for this PR to discuss. I'll address the class name and flip the RGB to bgr.

@pchote
Copy link
Member

pchote commented Nov 12, 2023

There are actually only a handful of true 16-bit sprites (the rest are 8 bit sprites with frame-specific palettes), and this parses them wrong! (so does my branch, FWIW). See frames 4300 - 4309 in the 1.06 patch's DATA.R16, which are supposed to be sidebar icons (cross-reference against DATA.R8).

I suspect this is going to be related to the imageHandle which is probably pointing somewhere else for these sprites.

I'm looking into this as part of resurrecting my https://github.com/pchote/OpenRA/tree/d2k-r16 branch with the goal of making d2k use the improved colour sprites by default. I'd like to propose that I take over this feature to avoid rebase conflicts.

@pchote
Copy link
Member

pchote commented Nov 13, 2023

On closer inspection, all of the frames that DATA.R16 has as 16bit appear to be duplicates of other frames that have the same data in 8bit! This suggests that the original game may not have used those frames at all and they just contain garbage.

The tileset R16 files do indeed contain valid 16bit data.

@pchote
Copy link
Member

pchote commented Dec 6, 2023

Superseded by #21240!

@pchote pchote closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants