Psionic tutorial crashes on using the heat token

[verhoevenv]

No description provided.

[Enet4]

I failed to reproduce that issue from here. Does it happen whenever you use the heat token? I've tried using it both in and out of the freezing area in the tutorial.

[verhoevenv]

Yeah, it happens whenever I use the heat token, at least in the tutorial. I have yet to try in another mod.

It crashes on WinMain.cpp:1940, where it wants to spawn the particles for a weapon. Apparently the weapon index (21 I think, not sure that is what it's supposed to be) is out of bounds for the vector mod.general_weapons.

Strange it doesn't crash on your setup...

[verhoevenv]

It happens only during the tutorial, not during the main game. Something wrong in the mod files? Why doesn't it crash for you, then, Enet4?

[Enet4]

That's a curious question. I have a hunch though: I'm not using the provided runtime files, I took them from another installation of the original Notrium 1.3.4.5. Give those a try.

[verhoevenv]

I believe I copied the files in the repo from an original 1.345 too. Can you check if there's a diff between your version of the runtime files and the ones in the repo?

[Enet4]

I confirm that the runtime files in the repo also cause this issue. Comparing them shows that the data files of the Tutorial mod have a slightly larger size in my version. Trying to compare them with meld however, shows that all files are identical. I've also looked into the textures, and they seem to be the same as well. So this really is a strange issue. I wonder if we can generate a knowledge base of all runtime data and export it to a file, in order to see what's different. Regardless, allow me to upload my version of the files.

[Enet4]

Ok, here are my files. It's most likely larger because I've been working with other mods and not all of them categorize their media contents. https://dl.dropboxusercontent.com/u/13392707/notrium_runtime.zip
Meanwhile, I think we should try to find an explanation to why the program crashes on that position. I solved another bug by asserting invariants along some parts of the code. I wonder if this approach is feasible here.

[verhoevenv]

Your files indeed don't show a difference for the tutorial. There's a few other differences though, not sure where they come from, but I don't think it really matters.

The bug still happens for me with your data files.

[verhoevenv]

On a Notrium 1.3.4.5 installation (not OpenNotrium), the Psionic tutorial doesn't show particle effects for the heat token, while on the main game, it does show particle effects. So yeah, maybe some index-out-of-bounds behavior that crashes in Visual Studio but doesn't crash for other compilers?

[verhoevenv]

I believe this is happening: items.dat specifies effect 23 with parameter2 being 21 for the heat token. Parameter2 gives the weapon id of which to use the effects. But weapon 21 doesn't exist in the Tutorial data files.

And from http://www.cplusplus.com/reference/vector/vector/operator%5B%5D/
Portable programs should never call this function with an argument n that is out of range, since this causes undefined behavior

Which might explain the differences in behavior for different compilers.

[Enet4]

Yes, that should be it. The at member function is safer, but would introduce some overhead if not replaced in the release build. Is it worthwhile to implement a macro'd support function and replace all vector access with it?

[verhoevenv]

Hmm. Maybe. To be honest, I don't think we will feel the speed penalty of compiling with at, but only trying it and measuring it can prove that - and I guess we would need a macro or something for that.

I'll see what the difference is on Visual Studio, where bounds checking is on by default but can be disabled.

An alternative to the macro is to use debugging flags to enable bounds checking in GCC. I think _GLIBCXX_DEBUG should do the job but that's only from reading the internet.

[verhoevenv]

Been thinking this through for a while. I don't think the difference in behavior of operator[] and at is the big thing to fix. Windows comes with bound-checked operator[] and I haven't had many problems.

I would suggest some other fixes though, ranked on scope:

1. Fix this issue by fixing the data files
2. Fix the bigger issue of data files being able to cause undefined behavior by doing some sort of validation on the data files, either with a different program or inside Notrium after they are loaded. (Validation needed on data files #27)
3. Fix the even bigger issue of the data files being spawn of Satan by moving to a proper scripting language. (Data file format is unreadable #28)

Any other thoughts on this?

[Enet4]

Here's what I think.

• We should obviously do (1).
• Doing (2) also makes sense, even if the verification is performed once, on first use.
• Yeah, good idea on (3). I will move to Data file format is unreadable #28 to talk about it.

While dealing with these fixes, we may refactor the code in order to improve robustness and encapsulation.