Trying to make this code a bit less ugly. It has been
hacked up by basically every Japanese and Chinese
version in different ways. This is a start to revert some
of the hacks.
The text display glitches in many different ways, since the
original devs did some pretty ugly hacks that we have to
emulate somehow. This commit fixes a couple of things
(spacing, pixel/line overdrawing, shadow, original
linebreak style). There are still several things that have to
be addressed separately, like various text colors, the
whole character portrait/stats pages, etc.
This reverts commit 1030283cf44605fa385fa4cb679d58cf6b938dbd.
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.
