This reverts commit 1030283cf4.
Giving this some more thought, it seems the safer thing to do. In practive, it shouldn't matter. I haven't encountered a single situation with an emty string...
Reported by GCC 12:
../scummvm/engines/kyra/script/script_hof.cpp: In member function 'int Kyra::KyraEngine_HoF::o2_defineSceneAnim(Kyra::EMCState*)':
../scummvm/engines/kyra/script/script_hof.cpp:875:32: warning: comparing the result of pointer addition '(((const char*)script->Kyra::EMCState::dataPtr->Kyra::EMCData::text) + ((sizetype)READ_BE_UINT16((((const void*)script->Kyra::EMCState::dataPtr->Kyra::EMCData::text) + ((sizetype)(((int)((Kyra::KyraEngine_HoF*)this)->Kyra::KyraEngine_HoF::<anonymous>.Kyra::KyraEngine_v2::<anonymous>.Kyra::KyraEngine_v1::emcSafeReadStack(script, 12, 875, ((const char*)"../scummvm/engines/kyra/script/script_hof.cpp"))) << 1))))))' and NULL [-Waddress]
875 | if (stackPosString(12) != nullptr)
Thanks to benoit-pierre for finding this.
(see PR #3448, https://github.com/scummvm/scummvm/pull/3448)
I chose a different approach for the fix, since I wanted an easy way of detecting/preventing this for similiar cases and I also didn't want to have so much extra code in that particular script function.
Currently const char * points into some random buffer with unclear
lifetime.
Instead use Common::String which ensures lifetime as long as a reference is
held.
This also removes the need of copying into existing buffers in HoF and MR.
UnkBuf* are gone as well.
Co-authored-by: athrxx <athrxx@scummvm.org>
* KYRA: don't attempt to execute functions out of bounds
This fixes crash in Siberian Goblin translation of HoF on scarecrow scene
* KYRA: Add sanity check not to execut out of bounds
* KYRA: Support Russian LoK translation by Siberian Gremlin
* Mention source of engine data translation
* Mark Russian CD as fan translation to force subtitles
- add support for the Sega Font and add text display methods at least for the sequence player
- finish sequence player (add all missing opcodes and other missing code portions)
- some improvement to the resource class
- some renaming and cleanup
This reverts the change from 1d5fd780 which was clearly wrong and caused this new bug (no idea what I was thinking there). To prevent the revival of bug #3721 I now added the proper code for a fix after tracking the whole bug with the DOSBox debugger.
The invalid table access takes place in several more locations. I missed the location that is actually responsible for the reported bug in my last fixing attempt (due to the fact that this bug is based on undefined behavior that results from the invalid mem access: in MSVC the bug is nonexistent, with GCC builds it seems to depend on whether they're optimized or not). The location that actually requires the fix is in KyraEngine_MR::enterNewSceneUnk2(). I have now fixed all locations where the table can be incorrectly accessed. The other locations have been marked with comments.
The optional green potion that can be made at the alchemists' crystals can lead to invalid memory access. The animation frames aren't properly cleaned up like in the snake poisoning sequence.
I just add the same handling as as workaround.
The assumption till now was that PC-9801 games in 16 color mode will always use text mode for text display. However, EOB1 uses text mode for intro and ending sequence and graphics mode for ingame.
Add new font variant for this and also do some cleanup, since the text displayer code really needs it.
(The problem is that at least 3 different Japanese publishers did the Japanese ports for the various games and every one of them did his own hacks for his specific target.)
Cameron Cawley
Le Philousophe
athrxx
eientei
Orgad Shaneh
Eugene Sandulenko
sluicebox
Vladimir Serbinenkoandathrxx
Dmitry Gladkov
mataniko
Bastien Bouclet