IWD2: crash when starting a new game #2082
Comments
|
Any group or a specific one? |
|
I clicked the first one from the top, not sure what it is named in English. |
|
Lady's Lament. Works here in English. Try breaking in |
|
Must be way up the stack, will have a look: |
that gets the data from here: gemrb/gemrb/core/Interface.cpp Lines 3081 to 3091 in 60dafac Wrong stream position? Will investigate tomorrow. |
|
It's not exactly related to the group choice, for me it's any new game. I was just too lazy or ran into other trouble when trying to do that manually. |
MarcelHB
changed the title
|
Yeah, makes sense, since you hit it with the basic goblin in the area. I see 10gob.cre does have a clean inventory, but one of the items is indeed randomized: 00rtgob1 will trigger a substitution. Is the resref bad for you already when entering |
Bad input from the outside: |
|
It modifies itm, so make sure to check before ResolveRandomItem runs. |
|
Right, |
|
|
|
So a problem in 2daImporter, I guess we still don't trim properly despite all the tests. |
|
Eh, |
|
As far as I can see, here you wanted I was looking through all the SKILLBRD.2DAs and I found some in PST and BG1 that match your commit description, but then again, they only have a single column? For the 2DAImporter, that doesn't matter anyway since it cuts the trailing occurrences over the number of columns, as they are seen from the header. In contrast this case here has 40 columns but 20 values + trailing space making the 21st, so that cut off is meaningless and we'll carry that around. So a fix is probably violating that test case, but if it's only relevant for some edgy 2DA file, I guess we are fine anyway. There is also a test for that 2DA case. |
|
Yeah, that behaviour matches the other odd explode cases. The importer got fixed in 8a77bde, since this could only happen in the first line IIRC. Since there's no way a 2da should be able to return an empty string except via an empty default value (just judging from our implementation, I doubt it's intentional), we could always just return the default value if the cell is empty. Which is something we can do at construction time and not repeat at each lookup. |
|
Shouldn't we clean up something here? I see three points:
|
|
|
That looks like the least intrusive way to fix the 2DAImporter without breaking anything else: diff --git a/gemrb/plugins/2DAImporter/2DAImporter.cpp b/gemrb/plugins/2DAImporter/2DAImporter.cpp
index 22ee5e3ee..ed93a9fe7 100644
--- a/gemrb/plugins/2DAImporter/2DAImporter.cpp
+++ b/gemrb/plugins/2DAImporter/2DAImporter.cpp
@@ -83,6 +83,11 @@ bool p2DAImporter::Open(std::unique_ptr<DataStream> str)
auto sv = StringView(&line[pos + 1], line.length() - pos - 1);
rows.emplace_back(Explode<StringView, cell_t>(sv, ' ', std::max<size_t>(1, colNames.size() - 1)));
+ auto& row = rows.back();
+ // Explode may have returned trailing space as the last but empty item
+ if (!row.empty() && row.back().length() == 0) {
+ row.pop_back();
+ }
}
assert(rows.size() < std::numeric_limits<index_t>::max());Changing |
|
Looks fine. 👍🏾 |
Is potentially invalid UTF-8 and will then crash libfmt. cf. gemrb#2082
Bug description
In IWD2, when starting a new game, by group or manual setup,
\r¡║\r¡║\r) as item name as seen in the stack below,Steps to reproduce
Expected behavior
Can start a game.
Screenshots
GemRB version (check as many as you know apply)
Video Driver (check as many as you know apply)
OPENGL_BACKENDenabledOPENGL_BACKENDenabledThe text was updated successfully, but these errors were encountered: