ImHex
ImHex copied to clipboard
[Bug] The "parent" keyword isn't always parsed in patterns
Operating System
Windows
What's the issue you encountered?
When executing a pattern, the parent
keyword is not always parsed correctly, leading to compile or runtime errors.
u8 mip[parent.mipInfos[parent.i].decompressedSize];
works. parent.i += 1;
causes a compile error. u8 mip[123] [[name(std::format("mip{}", parent.i))]]
causes a runtime error.
How can the issue be reproduced?
Use the attached pattern on the attached binary file.
ImHex Version
v13.33.1
ImHex Build Type
- [ ] Nightly or built from sources
Installation type
Portable zip from the GitHub Releases page
Additional context?
[Pattern: id Tech 7 BImage.hexpat] - [Binary file: combat_shotgun.tga$bc3$streamed]
there may be no problem parsing parent in patterns in your example code.
u8 mip[123] [[name(std::format("mip{}", parent.i))]]
the problem here is that the name attribute adds another level to parent. To access i you need to use parent.parent.i and it works fine.
parent.i += 1;
whatever the problem is with this, it has nothing to do with parent. in general any statement like (any name).(any member)=(any value) inside a struct will give the same error. In this case accessing i in from the parent to increment it is completely unnecessary as it can be incremented in its own structure using i+=1. (btw, using hidden in a local variable has no effect since local variables are always hidden)
I looked at your code in more detail and realized that you were trying to use i as an indication of the current array index. ImHex has a special function that returns the current array index in std::core::array_index(). with a small modification to your previous posted pattern (that tried to create an array of negative size) I was able to read the entire input file using the following changes: in ImageMip
u8 mip[parent.mipInfos[parent.i+std::core::array_index()].decompressedSize] [[name(std::format("mip{}", parent.parent.i+std::core::array_index()))]];
in Image
ImageMip mip[header.mipCount - header.streamDBMipCount];
this way i doesn't need to be updated since it can't be updated from the child which is why array_index function was created for.
[...]
u8 mip[123] [[name(std::format("mip{}", parent.i))]]
the problem here is that the name attribute adds another level to parent. To access i you need to use parent.parent.i and it works fine. [...]
I guess that that makes sense (especially for this
pointing to the currently-being-made pattern/variable), but it feels odd considering that u8 x = 10; u8 mip[123] [[name(std::format("mip{}", x))]];
doesn't need parent.
to access the x
next to it.
Seeing as you don't need parent.
in attributes when referencing other patterns/variables in the same struct, the first parent.
"layer" is essentially useless*; it might be more intuitive if the Pattern Language automatically "adds" an extra parent.
layer within attributes (so that parent.var
refers to the same thing both inside and outside of attributes).
* If you specifically want to refer to the struct containing the pattern/variable currently being created, then this.parent
should work, so there won't be any lost functionality by auto-adding an extra parent.
layer.
[...]
parent.i += 1;
whatever the problem is with this, it has nothing to do with parent. in general any statement like (any name).(any member)=(any value) inside a struct will give the same error. [...]
I found that out later: https://github.com/WerWolv/PatternLanguage/issues/87
But worth noting: Due to the "you need an extra parent.
in attributes" thing above, plus WerWolv telling me that parent
might not be parsed correctly (and telling me to make this GitHub issue about it), I thought that parent
was just a little iffy in general.
[...] ImHex has a special function that returns the current array index in std::core::array_index(). [...]
I've since edited my pattern file to use that (or more specifically u32 i = std::core::array_index() + parent.header.streamDBMipCount
within the ImageMip
struct, to use just i
later throughout the struct).
I just thought that it would've looked cleaner to do it like I was trying to do at first, considering that i
is supposed to start at a non-zero value here and std::core::array_index() + parent.header.streamDBMipCount
is somewhat of a "mess" to look at.
but it feels odd considering that u8 x = 10; u8 mip[123] [[name(std::format("mip{}", x))]]; doesn't need parent. to access the x next to it.
You are right that it doesn't need a parent there but note that it also works with parent.x equally well. I suspect that a parent layer is being skipped in the case of attributes referring to members of the parent struct, at least that's the only explanation I can think of of why it works with and without parent, considering how it needs two if the member is on the parent struct.
Don't get me wrong, I'm not saying that parent works as it should, I just don't see the problem in the example you posted. parent feature has been plagued with problems because it is a hard feature to implement. At one point creating arrays added a parent layer, but it looks like that's not the case anymore, so you may be right about it being fishy, but I still don't understand fully how it is supposed to work in reality to state that it is not working correctly.
std::core::array_index() + parent.header.streamDBMipCount
is somewhat of a "mess" to look at.
yes, I suppose it is. There are ways to make it look better (for example, in my version I use parent.i
instead of parent.header....) like writing a function or using a macro define, but in the end array_index() is the only way I know of to obtain a working current index and I tried lots of ways a while back.