Calling RandomSource::getRandomNumber() results in divide by zero
exception when called with max = 4294967295 as max + 1 overflows to zero
and the subsequent modulus operation causes the exception.
Certain game saves in Bladerunner trigger this.
This has been discussed on the mailing list. While there wasn't much
input, the consensus was that it should be allowed for consistency with
std::string.
While most engines don't do this, it is possible that some engines will
break because of it. (A few have already been fixed.) But it will also
repear the currently broken ADL engine. For now, the warning() message
will remain so that it can be investigated on a case-by-case basis.
This is currently only (partially) implemented for the 16-color Mac
versions of Loom and Indiana Jones and the Last Crusade. The text is
still drawn in color, since that's rendered separately, but I'm
committing this now while it still works.
This might also become a return to launcher, but I've somehere read the hint that
the event recorder was only meant to play back one game per session. We'll see.
the custom engine events were missing and the default event manager transforms the backend events
. Which means that the event recorder is not persisting the low level key press/release events
but the already transformed custom engine events of the keymapper
This is not the best solution as it removes the warning when this
code is used by devtools such as create_titanic, but getting this to
link would need inclusion of textconsole.o via shim and then still
breaks on g_system, so fastest solution to restore build.
While it's not needed to build ScummVM, apparently some of the other
tools that use String do not include textconsole.h and complain that
warning() is undefined.
Some engines do this, and it's not really known what should happen. We
need to decide on a behavior, and stick to that. This warning is to help
find where it happens.
From my own tests, it can happen in at least ADL, Avalanche, Chewy,
Director, Blazing Dragons, MacVenture, MADS, Prince, Stark, Startrek,
and Trecision. Only ADL is known to be broken by the current behavior.
The result of my test can be found at
http://www.update.uu.se/~d91tan/tmp/string.txt
../scummvm/gui/options.cpp: In member function 'virtual void GUI::OptionsDialog::apply()':
../scummvm/gui/options.cpp:647:82: warning: comparison of integer expressions of different signedness: 'uint' {aka 'unsigned int'} and 'int' [-Wsign-compare]
647 | else if (scalerPlugins[defaultScaler]->get<ScalerPluginObject>().getFactor() != g_system->getDefaultScaleFactor())
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
==3124361== Invalid write of size 8
==3124361== at 0x483F803: memmove (vg_replace_strmem.c:1270)
==3124361== by 0x4DBF61: SurfaceSdlGraphicsManager::grabOverlay(void*, int) const (surfacesdl-graphics.cpp:1753)
==3124361== by 0x482051: ModularGraphicsBackend::grabOverlay(void*, int) (modular-backend.cpp:215)
==3124361== by 0x434EE1: GUI::ThemeEngine::clearAll() (ThemeEngine.cpp:376)
==3124361== by 0x40128E: GUI::EventRecorder::preDrawOverlayGui() (EventRecorder.cpp:558)
==3124361== by 0x481DB2: ModularGraphicsBackend::updateScreen() (modular-backend.cpp:173)
==3124361== by 0x559967: Graphics::Screen::updateScreen() (screen.cpp:62)
==3124361== by 0x55991C: Graphics::Screen::update() (screen.cpp:56)
==3124361== by 0x38AFC7: TwinE::TwineScreen::update() (twine.cpp:126)
==3124361== by 0x3B8759: TwinE::Screens::adjustPalette(unsigned char, unsigned char, unsigned char, unsigned int const*, int) (screens.cpp:150)
==3124361== by 0x3B8A89: TwinE::Screens::fadeToPal(unsigned int const*) (screens.cpp:207)
==3124361== by 0x3B8403: TwinE::Screens::loadImage(int, int, bool) (screens.cpp:80)
==3124361== Address 0x31453050 is 16 bytes after a block of size 512,000 alloc'd
==3124361== at 0x483AB65: calloc (vg_replace_malloc.c:760)
==3124361== by 0x55B38C: Graphics::Surface::create(unsigned short, unsigned short, Graphics::PixelFormat const&) (surface.cpp:75)
==3124361== by 0x551111: Graphics::ManagedSurface::create(unsigned short, unsigned short, Graphics::PixelFormat const&) (managed_surface.cpp:153)
==3124361== by 0x4352D5: GUI::ThemeEngine::setGraphicsMode(GUI::ThemeEngine::GraphicsMode) (ThemeEngine.cpp:453)
==3124361== by 0x434A52: GUI::ThemeEngine::init() (ThemeEngine.cpp:324)
==3124361== by 0x43501B: GUI::ThemeEngine::refresh() (ThemeEngine.cpp:394)
==3124361== by 0x405780: GUI::GuiManager::screenChange() (gui-manager.cpp:603)
==3124361== by 0x405C6B: GUI::GuiManager::processEvent(Common::Event const&, GUI::Dialog*) (gui-manager.cpp:677)
==3124361== by 0x404EBA: GUI::GuiManager::runLoop() (gui-manager.cpp:429)
==3124361== by 0x3FD847: GUI::Dialog::runModal() (dialog.cpp:77)
==3124361== by 0x36D747: launcherDialog() (main.cpp:106)
==3124361== by 0x36FF92: scummvm_main (main.cpp:552)
It looks like the _videoMode.overlayHeight in SurfaceSdlGraphicsManager::grabOverlay and ThemeEngine::_backBuffer::h are somehow out of sync after
starting the game in a different resolution as the gui was started with. So the overlayHeight is updated - but the backbuffer (Surface) is not resized.
This is with event recorder being active - right after starting the game and switching the resolution.
==1313424== Conditional jump or move depends on uninitialised value(s)
==1313424== at 0x682225: Common::wrapCompressedReadStream(Common::SeekableReadStream*, unsigned int) (zlib.cpp:498)
==1313424== by 0x46CDB9: DefaultSaveFileManager::openForLoading(Common::String const&) (default-saves.cpp:134)
==1313424== by 0x68334D: Common::PlaybackFile::openRead(Common::String const&) (recorderfile.cpp:74)
==1313424== by 0x444558: GUI::RecorderDialog::updateList() (recorderdialog.cpp:206)
==1313424== by 0x4446BD: GUI::RecorderDialog::runModal(Common::String&) (recorderdialog.cpp:218)
==1313424== by 0x3DF0E5: GUI::LauncherDialog::recordGame(int) (launcher.cpp:461)
==1313424== by 0x3E0397: GUI::LauncherDialog::handleCommand(GUI::CommandSender*, unsigned int, unsigned int) (launcher.cpp:671)
==1313424== by 0x400BF8: GUI::CommandSender::sendCommand(unsigned int, unsigned int) (object.h:55)
==1313424== by 0x42DAB8: GUI::DropdownButtonWidget::handleMouseUp(int, int, int, int) (widget.cpp:497)
==1313424== by 0x3D3A37: GUI::Dialog::handleMouseUp(int, int, int, int) (dialog.cpp:228)
==1313424== by 0x3DB72C: GUI::GuiManager::processEvent(Common::Event const&, GUI::Dialog*) (gui-manager.cpp:668)
==1313424== by 0x3DA9EA: GUI::GuiManager::runLoop() (gui-manager.cpp:429)
Happens when you start with the event recorder compiled into scummvm and open the dialog to start
a new record.
