StoneSense crashes on DFHack 0.47.03-beta1 #68
Comments
|
I'm getting this error on running The version of dfhack I'm using is The issue seems to be exactly the same as: DFHack/dfhack#1521 (comment) |
|
Same issue on |
|
Yeah, Stonesense hasn't been meaningfully changed in almost a year. I'm not sure if this issue is due to something that changed in 0.47 that DFHack hasn't caught yet (which would be surprising at this point, but fixable), an issue that needs to be addressed on the Stonesense side (which would be unlikely to happen any time soon unless someone steps up to maintain Stonesense), or something related to newer systems (in which case I would expect at least some more detail in stderr.log). You wouldn't happen to have a stack trace from this to narrow it down, would you? If you have GDB installed, you could run |
|
It's a memory corruption from something being loaded improperly as soon as stonesense tries to initialize its assets:
the filepath is : stonesense\items\index.txt EDIT: grei_items.png is the culprit. |
|
I think it crashes because of this :
What I don't understand though, is why the array is not enlarged beforehand. |
|
DFHack/dfhack#1521 (comment) also reaches the same conclusion. |
|
I'll try to get this fixed. |
|
29b430d broke this by removing the part where it makes sure that the item array is the right length. |
|
Not sure you're quoting the correct commit, as the one you quoted mentions TerrainMaterialConfiguration, whereas this issue is with ItemConfiguration ? (Even though the logic does seem similar and does seem to have been broken) |
|
Oh, he changed a bunch of things, but I realized that his specific chances weren't what was causing it. Still trying to make sure I caught everything, 5b7e774 may fix it. |
|
Your change does two things :
This first change is not really necessary. EDIT: It is necessary
I think that's the solution to our issue. Before, if we encountered an item type which enum was going up to, let's say, 31, then we would resize the array to 31 instead of 32. Which means we could never go up to the cell at index 31! That was clearly a bug that needed to be fixed. I shall end with a sour comment about how once again when looking at C++ code, there's a fatal bug in a simple operation because someone thought they were too smart to just use off-the-shelf, one-liner array primitives... and made a mistake in their custom 10-lines replacement. ;) |
|
I've tested it. It works. ...while this crashes : |
|
This is why I hate C++. |
|
|
|
Ha ha I love how the hardened C++ dev answers "this would be less error-prone" rather than "look, those 10 lines are now one, I used a built-in function". |
|
I mean, re-implementing built-in operations usually is more error-prone regardless of the language. I'm more inclined to blame the logic here rather than the language itself. Looking again at the two snippets you posted, the main difference I'm seeing is that the first one (which you said works) resizes config to be exactly the desired size, while the second one resizes it to be at least the desired size - if it was larger before, it will still be larger. Just by looking at it, I don't see any way that it can crash by itself, but it could probably affect other parts of Stonesense in difficult-to-predict ways. (I would have guessed that the first snippet would be more likely to result in a crash later on, but it's hard to say without more context.) Casting currentsize from a size_t to a uint32_t (which is sometimes smaller) is inadvisable, but probably unlikely to trigger any issues for smaller vectors. Edit: oh, I think I see the issue in the second snippet - |
|
I'm all for using the actual builtin function for this. |
|
Also DFhack really should have a ENUM_SIZE macro. |
|
Ah yes, indeed, it should be while(config.size()<currentSize ) , not while(currentsize < config.size()). But that can be dumped altogether if the one-liner is used. So yes, I vote for that. @RosaryMala not to be lazy but can you check that this way of doing things is not used in other XXXConfiguration.cpp files? |
|
Yeah, I'll take a look at the rest of the things and make sure they're all working. |
|
5b7e774 My first time playing DF =D Also I see that the keyboard control issue is still unsolved, and won't be? |
|
We would probably have to move Stonesense to a standalone process to make keyboard input work (that would have other benefits, like crashes not bringing down DF, but would also need someone familiar with Stonesense and willing to put in the effort). For reference, I'm pretty sure that the automated builds from https://dfhack.org/builds/ (like this one) include Stonesense. |
|
Until this is fixed, could someone provide a brief walkthrough/workaround on how to get this working with say, the LNP? It would be much appreciated <3 |
|
If you follow my automated builds link from above, you should get to a page like this (this page may be outdated by the time you click the link, though, so I recommend using the first link instead). Under "Build artifacts", choose the build that matches your DF installation (e.g. "Windows64" for 64-bit Windows DF). Once that file has downloaded, extract "hack/plugins/stonesense.plug.(dll, so, or dylib)" from it and replace the file in your existing hack/plugins folder with it. Generally, when doing this with Stonesense, you should also replace the "stonesense" folder with the downloaded copy, but that shouldn't be necessary in this case. When you start DF, you'll probably see a warning that the plugin was built for a different DFHack build, but as long as it starts with "Warning", it's safe to ignore. |
|
Perfect, thank you. I can confirm this works with PeridexisErrant's Starter Pack 0.47.04-r06. |
|
Doesn't work for me :/ |
|
You need to upgrade to DFHack 0.47.04-r1 (which involves upgrading to DF 0.47.04) - the plugin will only work for that version. |
|
Assuming 5b7e774 fixed this, this should be addressed in 0.47.04-r2. Feel free to open a new issue if problems continue. |
Hello,
A crash occurs upon loading StoneSense in DFHack 0.47.03-beta1 on Windows 10.
Here is my
stderr.log:EDIT: This also seems to happen on 0.44.12-r3
The text was updated successfully, but these errors were encountered: