Crash fixes, editor improvements, cross-platform fixes and 27 new levels - #16
Open
Upabjojr wants to merge 26 commits into
Open
Crash fixes, editor improvements, cross-platform fixes and 27 new levels#16Upabjojr wants to merge 26 commits into
Upabjojr wants to merge 26 commits into
Conversation
Five related defects surfaced while getting the game running against OGRE 13.6.5 on X11. Pump window events. Ogre::Root::startRendering() no longer pumps them (Ogre::Bites does that for applications built on its context, which we are not), so ConfigureNotify never arrived: windowResized() ran only once at startup and CEGUI's display size and OIS' clipping rectangle went stale as soon as the window was resized. Drive the render loop directly and call messagePump() every frame. Exit cleanly when the window is closed. windowClosed() destroys the mode manager, but frameStarted() dereferences it before testing the exit flag, so the next frame segfaulted in _fireFrameStarted(). This was unreachable before, as close events were never delivered. Request exit from windowClosed() and check it before rendering. Hide the system cursor while grabbing. When grabbing, OIS tracks the pointer by accumulating relative motion and warps the real pointer back to the window centre near the edges; leaving that cursor visible showed it drifting away from the one CEGUI draws. Keep the configured video mode. initVideoConfig() dropped any video option whose value was not among the render system's possible values, but the video mode also sizes the window when running windowed, where it need not be one of the fullscreen modes. It was silently falling back to the minimum size. Raise the minimum window height to 660. MenuMain.layout stacks its seven buttons around the vertical centre with fixed offsets and needs 660px; below that the game clipped the "Quit" button off its own menu. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scrolling ran past the map edge on one side while stopping short of it on the opposite side, in gameplay and in the map editor alike. updateCameraFrameTime() clamped the camera node's own position to [0, mapSize], but the camera is pitched DEFAULT_X_AXIS_VIEW (25 degrees) off vertical, so the ground point it looks at lies z * tan(pitch) ahead of it: between 1.4 and 7.5 tiles for z within [MIN_CAMERA_Z, MAX_CAMERA_Z]. Clamping the position therefore shifted the whole reachable view window by that offset, overshooting the edge the camera faces and falling short of the one behind it, for a total asymmetry of twice the offset. Clamp so the view target stays inside the map instead, deriving the offset from the camera's derived direction and the height it is about to end up at, the same way getCameraViewTarget() does. Keeping the offset signed leaves the limits correct under camera yaw, not just at the default orientation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Playing for a few minutes reliably ended in a segfault inside GameMode::mouseMoved(), dereferencing a freed Creature from RenderManager::rrNormalizeAmbient(). InputManager::mHighlightedCreature is a bare pointer to the creature the mouse hovers, kept so its ambient can be restored once the mouse leaves it. Only GameMode::mouseMoved() and Creature::setPosition() ever cleared it, and neither runs when the creature dies: client-side deletion goes through deleteYourself() and GameMap::processDeletionQueues(), which touch nothing in InputManager. Hovering a creature that then died left the pointer dangling, and the next mouse move read freed memory. Clear it from ~Creature(). Three null dereferences in the same area, none of them observed firing: Creature::setPosition() dereferenced getPositionTile() twice without checking it, but that returns null whenever the creature sits outside the map, as it does while held in the keeper hand or carried. ODFrameListener::frameStarted() dereferenced getOverlayStatus() for every creature, which is null while a creature's mesh is not created -- Creature::update() checks it for exactly that reason. The two loops around the render target update also walked the creature list twice and paired the results by index, which only holds while no creature gains or loses its overlay in between; remember the overlays actually hidden instead. Tile::setEverVisible() was declared to return bool and returned nothing, which is undefined behaviour. Its only caller ignores the result, so this was harmless in practice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The game read its configuration and the shipped levels from the data
folder it was configured with at build time. When that folder was never
installed, there was nothing to fall back on but the current working
directory, which is why running from a source checkout was the only way
to start it.
Compile config/ and levels/ into the executable and extract them into
the user data folder instead, under a gamedata/ subfolder:
~/.local/share/opendungeons/gamedata/config/*.cfg
~/.local/share/opendungeons/gamedata/levels/{skirmish,multiplayer}/
cmake/GenerateBuiltinData.cmake turns the two folders into a generated
translation unit at build time, declared by source/utils/BuiltinData.h.
ResourceManager writes out every file that is not already there and then
reads the configuration and the shipped levels from that folder, so the
game no longer depends on a system wide folder existing or being
readable. Existing files are never overwritten, since the folder is the
player's and may have been edited on purpose; a digest of the embedded
data is stamped alongside them so that a build carrying different
defaults is reported rather than silently ignored. --gamedata reads them
from somewhere else, which is the convenient way to run against an
edited source tree.
Saving a level in the editor now writes to the user levels folder unless
the level already lives there, rather than over the file it was loaded
from. Editing one of the shipped levels used to modify it in place, so
the original could not be played again. This restores the intent of the
code that was left commented out in askSaveMap, comparing normalised
absolute directories rather than searching for substrings in paths.
Two supporting changes: the server mode options name a level, so they
moved to their own step that runs once the default data path is known;
and ODApplication now logs the resolved paths, which ResourceManager
cannot do itself because the log file sink does not exist yet while it
runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comparing the stamp written into the user data folder against the one this build carries reports any difference between the two builds, including one that has no effect: a build that only adds files writes them out normally, since extractBuiltinData() skips existing files rather than all of them. Shipping a new level would have warned that the folder was kept as it is, right after correctly putting the new level in it. Count the files that exist but hold something other than what this build would have written, and warn only about those. The stamp is still written, as a record of which build populated the folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Multiplayer, every player given exactly the same start:
Twin Fangs 65x65, 2 players, half turn rotation. A gem seam in the
middle, ringed in lava with a breach on either side.
Four Winds 81x81, 4 player free for all, quarter turn rotation, so
no corner is better than another. Gold spokes reach out
from a crossroads nobody can hold quietly.
The Broken Seal 73x73, 2 players, half turn rotation. A vault walled in
rock holds two hero portals that send waves at a keeper
picked at random, so neither player can rely on the
other soaking them.
Skirmish, each with something to achieve:
The Golden Vein 65x65, a mining race against one AI keeper. Mine
2000, then 6000, then 12000 gold, and destroy him.
Heroes at the Gate 65x65, no rival keeper, only escalating hero waves
from two portals down two defensible approaches.
Hold the temple and kill every last hero.
The Deep Catacombs 73x73, walled crypts on a grid. Claim 250 tiles,
then 600, against an AI keeper, while hero waves
arrive from both ends.
Equality on the multiplayer maps is structural rather than eyeballed: each
map is authored once and stamped through the rotations that generate it,
so a player's surroundings are the image of every other player's. The maps
are odd sized on purpose, so the rotation has a centre tile to fix; on an
even sized map there is no such tile and the middle cannot be symmetric.
Checked before committing: the whole tile grid maps onto itself under the
rotation with seat ids following it, early reachable gold is equal for
every player, every room tile is claimed floor owned by the room's seat,
each seat starts inside its own temple, and every temple can dig its way
to every portal and to every other temple without crossing water or lava.
All six load in the engine with no warnings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Multiplayer: Sunken Halls 97x97, 2 players, half turn The Iron Cross 97x97, 4 players, quarter turn Ember Basin 113x113, 4 players, quarter turn The Shattered Ring 113x113, 2 players, half turn, hero vault Heroes of the Deep 113x113, 4 players, quarter turn, hero vault Long Winter 129x129, 2 players, half turn The Eight Thrones 145x145, 8 players, every symmetry of the square Skirmish: Gold and Ashes 97x97, 1 rival keeper, mine 3000/9000/18000 gold The Lost Legion 97x97, hero survival, three portals The Great Warren 113x113, 3 rival keepers, claim 400 then 900 tiles Siege of the Deep 129x129, 3 rival keepers plus a hero vault The Endless Tide 129x129, hero survival, five portals, faster waves Kingdoms Fall 145x145, 7 rival keepers, claim 400 then 1000 tiles The first six maps were authored feature by feature, which does not scale to a 145x145 map without it turning into a field of undug dirt. These are built from three templates instead: the symmetry group decides how many players there are, and a seeded generator fills the authored wedge with rock, water, lava, caverns and gold seams. The seed is part of the map, so each one regenerates identically. Eight players come from using all eight symmetries of the square rather than just the rotations, so the eight starts are the same ground seen eight ways. The wedge is then an octant, which is why that map is the largest: the keeper's ground has to fit inside it without touching its own reflection. Filling a wedge automatically needs the wedge boundary respected, so scatter() places a blob only when the whole blob is inside it, not merely its centre. stamp() already refuses to let two stamps write different terrain to one tile, which is what keeps a generated map as symmetric as a hand authored one. All nineteen levels were checked the same way as before: the tile grid maps onto itself under the map's rotation, with the seat permutation derived from where the temples land rather than declared; early reachable gold is equal for every player, including all eight on the largest maps; rooms are claimed floor owned by their seat; and every temple can dig to every portal and every other temple without crossing water or lava. All nineteen load in the engine with no warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Playing on eventually ended in a segfault on the server thread, inside
MissileObject::doUpkeep(), reading address 0xf8. That is offset 248,
which is where Tile::mX sits.
doUpkeep() walks the tiles along the missile's path, remembering the
previous one in lastTile so that a missile stopped by a wall can be
placed on the tile it came from. lastTile starts null and is only
assigned at the bottom of the loop, so when the first tile of the path is
already a wall both branches of the wall case read through it. The loop
below that case already checks lastTile for null, which is what the wall
case should have done too.
That happens when a missile is launched straight at a wall, or when the
tile it occupies is filled in under it, both of which take a while to come
up: the log for this one shows it on turn 8858, right after
missile name=RenderedMovableEntity_Cannon_10_6102, hit wall on tile=[23,32]
which doUpkeep() logs two statements before the dereference.
There is no previous tile to fall back on in that case, so end the
missile where it already is.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clicking near the border of the minimap moves the camera there and leaves it shuddering against the map edge for the rest of the game. The minimap is a view of the ground around the camera, not of the map, so when the camera is near an edge the minimap shows ground past it, and a click there asks the camera to fly somewhere it cannot go: updateCameraFrameTime() keeps the point the camera looks at inside the map. The flight is never within the 0.25 stopping distance of its destination, so it never ends, and every frame it pushes the camera past the edge for the clamp to pull it back on the next one. Hence the bouncing, and hence it outlasting the click. flyTo() now aims at the nearest point on the map, which is where the camera was going to stop anyway. Two things back that up. The clamp moved into clampToMap() and is applied a second time just before the camera is placed, so the flight, the circle and the spline modes, which all add to the position after the first clamp, can no longer put the camera off the map even for the single frame it took to be pulled back. And a flight that brings the view target no closer than the frame before now gives up rather than press on, so no destination the clamp does not predict can hang the camera again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selecting a spell that is still cooling down puts a countdown next to the mouse pointer, "CreatureExplosion (2.34 s)". It only moves while the mouse does, so hovering an enemy waiting to explode it leaves the countdown reading whatever it did when the mouse last stopped, and still reading it once the spell is castable again. The text is written by checkInputCommand(), which runs on mouse move and on click. Everything else it displays depends on where the pointer is, so that was enough; the cooldown is the one thing that changes while the player does nothing. The value behind it is fine, Player::frameStarted() smooths it every frame, and the progress bar over the spell icon follows it correctly. Refresh the countdown every frame as well. The cooldown branch of checkSpellCast() moves into checkSpellCooldown() so that both callers share it, and because it is worth stating separately that this part displays and never casts, whatever the input state is. Once the cooldown ends, the countdown has to give way to whatever the spell puts there instead, which only the spell knows, so checkSpellCast() is asked once. Not when the input state is validated, since that is the state in which it casts the spell rather than describing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A creature walking towards a bridge when it is sold walks to the last tile before the water or the lava and stops there for the rest of the game. One standing on the bridge when it goes stops just as permanently, in the middle of the lava. Nothing tells a creature that the ground it planned to walk on has gone. It keeps the path it computed, steps onto the tile the bridge used to cover, and Creature::getMoveSpeed() returns the creature's lava speed there, which is zero for everything that does not live in it. The distance it may move in a frame is that speed times the frame time, so it stops where it is, its walk queue never empties, CreatureActionWalkToTile never completes, and the action that sent it walking never gets to choose again. It is a statue that can still be attacked. So look, once a turn, at whether the ground still holds: checkWalkPathIsStillValid() walks the remaining path and, if any tile of it has stopped being walkable, stops the creature and looks for another way to the same destination. If there is none, it stops there and the action that started the walk decides what to do instead, exactly as it would have done had the creature arrived. A tile that refuses passage because of the building on it, a closed door or a prison, does not count: those are meant to stop the creature, and it was already walking towards one knowing what it would find. checkStandsOnWalkableTile() puts a creature that is standing where it cannot stand back on the closest tile where it can. That is the only way out for it, since the null speed applies to its own movement as well. It only fires for water and lava with nothing built on them, which is exactly what a bridge leaves behind, rather than for anything that happens to give a null speed. Also stop the client dereferencing the entity named by an entityTeleported message without checking it knows it, since teleports are no longer limited to the arena gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The level file format has carried a creature's level since forever, and the game reads it back, but the editor always wrote 1: it spawned every creature straight from its definition and offered no way to say otherwise. Levelling a creature in a map meant editing the file by hand. L now raises the level the editor gives to creatures, shift L lowers it, both wrapping around the thirty levels a creature can have, and the status bar shows it next to the class the way it already shows the fullness, the seat and the class themselves. The level applies to the creatures spawned from then on, and also to whatever the hand is holding when it changes, which is what makes it possible to change a creature that is already placed: pick it up, set the level, drop it. Only the server knows what a hand holds, so the client sends the level and lets the server decide what it lands on. An empty hand is not a mistake, it just means the editor is setting the level of the creatures to come. Placing a creature also heals it. Levelling raises the maximum HP without healing, which is what levelling up in a game should do but not what an author placing a level 20 hero means, and the difference would have been saved into the map as a wounded creature. The status bar field is 20 pixels wider so that the longest class name and a two digit level still fit within it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C cycles forward through the creature definitions and wraps around, so reaching the class just before the current one means pressing it as many times as there are classes. Shift C now steps back the same way shift L lowers the level. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…iting Shift Y walks the seats backwards, the way shift C and shift L already walk the creature classes and the creature levels, so GameMap gains previousSeatId next to nextSeatId. None of that is discoverable. The status bar names the key next to each thing it shows, which is how anyone finds T, Y, C and L in the first place, but there is nowhere it could say that holding shift reverses them, and nothing at all mentions the camera keys or the point of view hotkeys. Help then Controls now opens a window listing all of them, grouped by what they act on. The Help menu had one entry, About, which is not connected to anything. The editor also never said which level it was editing. A new level is given a name in the dialog that creates it and then never shown again, and loading one from the recently used list gives no confirmation of what was opened. The name now sits under the menu bar. It is read every frame rather than once, because the client only learns it when the server sends the level over, which happens after the editor is on screen, and it changes again on saving under a new name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The macOS branch of ResourceManager::setupDataPath() does not compile: it builds the bundle path with applePath + "/", where applePath is a char array and "/" is another array, so it is adding a pointer to a pointer. Whatever else is true of the macOS build, it has not been attempted for a long time. The editor's load and save dialogs start in $HOME, which on Windows is only set if somebody has set it, so they opened on nothing. They now start in the folder the player's own levels are saved to, which ResourceManager already resolves per platform, and which is where a level being loaded or saved from the editor belongs anyway. That was getEnv()'s only caller, and its comment already said it should be doing something else on Windows. Neither of these is verified on the platform it concerns. There is no macOS in the build system beyond a single if(WIN32 OR APPLE), and the Windows CI is an AppVeyor file pinned to a branch this fork does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A level file written on Windows does not load anywhere else. Its lines end with a carriage return before the line feed, and only Windows takes that back off when reading, so everywhere else the carriage return stays at the end of the line. There it makes the last value on the line, or a section marker standing alone, into something the parser does not recognise, and loading fails outright: verified here by converting a level to CRLF, which turns into "The level file can't be loaded". Helper::readFile now strips it, which covers every level and every config file since they all come through there. The same level loads cleanly afterwards, as do all twelve config files converted the same way. Then the Windows specific parts: The window icon is installed with SetClassLong and a handle cast to LONG. A handle is 64 bits wide in a 64 bit build and LONG stays 32, so that does not compile there. SetClassLongPtr is the same call in a 32 bit build. The Discord link ran xdg-open, which is the freedesktop way and exists on neither of the other two. It now uses ShellExecute on Windows and open on macOS. The editor's file lists asked whether a name starts with a dot to decide whether a file is hidden. On Windows that is an attribute of the file rather than anything in its name, and the code that would have read it was commented out, so the "show hidden files" checkbox did nothing. It now reads the attribute, which needs the whole path rather than the file name. setupOgreResources treated a path as absolute if it starts with '/', which no absolute Windows path does. And the two places that make sure a folder ends with a separator only accepted '/', so a path ending in a backslash got a slash added after it. Only the level file fix is verified: it is the only one of these that can be reached from Linux. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Entering the editor a second time in one session kills the game:
CEGUI::AlreadyExistsException ... Failed to add Element named: Adventurer
to element at: EDITORGUI/Menubar/Creatures/PopupMenu4 since an Element with
that name is already attached.
EditorMode is built anew every time the editor is entered, and its constructor
fills the Creatures menu with one item per creature definition, the recently
used files menu and the Seats menu. The window it fills them into belongs to the
Gui and lives as long as the game does, so the second visit finds all of them
already there. CEGUI refuses a second child of the same name by throwing, and
nothing catches that before it reaches main().
Two of those three menus already had a function to empty them, used when their
contents change while the editor is running, but it was never called on the way
in. The creature list had none, so it moves out of the constructor into the same
install and uninstall pair as the others, and all three are now emptied before
being filled. GameMode has the same kind of list, of the seats in the game, and
already removes it in its destructor, which is why only the editor is affected.
Emptying detaches the old items without destroying them, which is what the
existing functions do: one of them is called from the click handler of an item
it is removing, and destroying a window in the middle of dispatching its own
event would not end well.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Entering the editor a second time now segfaults on its first frame, reading address 0x10 inside DebugDrawer::build() called from EditorMode::onFrameStarted. RenderManager makes the debug drawer along with the scene, in createScene(), which the ODFrameListener constructor calls once for the whole run of the game, and destroys it in its own destructor. EditorMode's destructor deleted it too, every time the editor was left. Ogre::Singleton clears its instance pointer as it goes, so the second editor session found nothing there and drew through it anyway: getSingleton() returned a null reference and build() read its manual object, which sits at offset 16 of a DebugDrawer, from address 0x10. The editor still clears the drawer of what it left behind, which is its own, and checks first, since at shutdown the render manager may already have taken it. This was hiding behind the exception fixed in the previous commit, which killed the game earlier in the same sequence, while EditorMode was still being built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selling the middle of a bridge leaves two stretches of bridge that are still one room. Everything done to a room is then done to both, and claiming is the one that shows: an enemy worker dancing on one stretch takes the other one too, across the lava it cannot reach. Rooms merge but never split. checkForRoomAbsorbtion() makes one room out of two that come to touch, and nothing does the reverse when tiles are lost, whether sold or destroyed. So Room gains checkForSplit(), which gathers the covered tiles into the groups that hold together, by the same neighbouring rule that decides two rooms are really one, and hands every group but the biggest to a room of its own. It runs after tiles are sold and after Building::doUpkeep() removes the ones whose hit points reached zero, which is the enemy digging through the middle of a room. Handing the tiles over follows absorbRoom(), which does the same thing the other way round. The new room gets a copy of what this one knew about each tile, and what was built on it. This one keeps the original marked destroyed rather than dropping it: a seat remembers which building covers each tile it can see and asks that building about it until told otherwise, so a room that forgets a tile it has handed over is asked about it anyway and reads through a null. Those tiles are not offered for repair, since a tile the other room covers cannot be built upon. What a room keeps for itself rather than per tile is its own business, so splitRoom() lets it give the new room its share. A bridge splits the value an enemy has to dance away, in proportion to the tiles. A treasury has the opposite problem: it counts the gold of every tile it has data for, not only the ones it covers, so the copy left behind has to be emptied or selling the middle tile of a treasury makes gold instead of costing it. It is not only bridges. Any room can be cut in two, and a room in pieces gets its active spots, its creature places and its centre wrong. Traps are claimable and tile based as well and have the same bug, but they build their tiles differently and are left alone here: Building::checkForSplit() does nothing unless a building knows how to split. source/tests/test_RoomSplit.cpp builds a treasury five tiles wide, fills it with 5000 gold and sells the tile in the middle. 4000 is left, the gold of the four tiles that remain. Losing what the moved tiles held would leave 2000, and copying it would make 6000. The test found both of those, and a crash, before this commit was worth making. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The label over a creature is not attached to it. It is placed on the screen, worked out each frame from where the camera sees the creature, so it is only over it for as long as something keeps putting it back. Three things stopped that from happening. GameMap::updateAnimations() returns without doing anything while the game is paused, and the labels are moved from there. The camera is not paused: it is updated in frameRenderingQueued() before the check that stops the frame when the game is. So pausing and then moving the camera left every label behind, over whatever ground its creature had been standing on. The same holds before the first turn. The labels are now put back in both cases, given no time so that nothing they say changes and no creature with more than one mood to show takes its turn while the game is not running. The label is placed from the bounding box of the creature's mesh, taken as Ogre holds it rather than derived. That box is from the last time the scene was updated, and the creatures are moved after that, during the frame, so every label was a frame behind whatever was moving. And nothing hid the label of a creature that had died. A dead creature lies where it fell for a few turns before it is taken away, wearing its level and its mood the whole time. The client can tell: an overlay health value of the last step means dead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Building with OD_BUILD_TESTING=ON has been broken for a while: the console interface test no longer compiled, and once it compiled it did not link, and once it linked it crashed. One layer at a time: addCommand() lost its description parameter at some point, with the form that takes one renamed to addCommandAux(), and the test still called the old one. ConsoleInterface.cpp includes pybind11/embed.h without using anything from it. The include drags Python symbols into the object file, which the game only carries because it links pybind11::embed; the test target does not, and has no reason to. The include goes away, which unbreaks the link and drops a spurious dependency. Executing a command no longer records it in the history: the history belongs to whoever owns the prompt, and the game console, which now runs its commands through Python, records what was typed itself. The test still assumed recording was implicit, so its history was empty and it dereferenced the boost::none that scrolling an empty history correctly returns. It now records each command the way the game does, and the scrolling expectations hold as written: nine commands in, the prompt round trip, nine commands back out. The whole suite passes, each test against its own server the way run_unit_tests.sh starts them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A seat remembers, for every tile of the game map and of the editor's copy paste container, what it last told its player about it. The two containers keep their states in two separate maps, filled once when each container is sized, and most of the code looks a tile up in the first map and falls back to the second. Four kinds of mistake had crept into that pattern, all in Seat.cpp: hasVisionOnTile() and notifyTileClaimedByEnemy() find the tile in the fallback map through one iterator and then read or write through the other, which is the end of the first map. That is a read past the end of a std::map whenever the server asks about a copy paste container tile. updateTileStateForSeat() looked the tile up with operator[] before doing the find with fallback. The lookup creates a default state for the tile in the game map's map even when the tile belongs to the container, so the find always succeeded, the fallback was dead code, and the update went to the stray entry rather than to the container's own. The stray entries also outlive the container whose tiles they are keyed by. setVisibleBuildingOnTile(), notifyBuildingRemovedFromGameMap(), tileMarkedDiggingNotifiedToPlayer() and exportTileToPacket() had no fallback at all, only the inserting operator[]. They now share one lookup helper that searches both maps and never inserts; the export keeps sending a default state for a tile it does not know, since that is what the insertion amounted to. Also two smaller ones found on the way, both in the render manager: initGameRenderer() hides the creature overlays around its first render target update with the same lockstep double loop, without null checks, that used to crash ODFrameListener::frameStarted(); it gets the same fix. And a range for in Building.cpp iterated the building objects through a reference of the wrong pointer type, converting every element to a temporary pair on the way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Multiplayer: Mirror Pass 89x89, 2 players, mirror symmetric The Salt Lake 105x105, 2 players, gold island behind water Crown of Embers 121x121, 4 players, hero vault The Four Gates 137x137, 4 players, the largest four player map Skirmish: The Breach 89x89, hero survival, two fast portals Thieves Gold 105x105, 1 rival keeper, race for the island gold Old Enemies 121x121, 1 rival keeper plus a hero vault The Last Gate 137x137, hero survival, six slow, heavy portals Two things are new in the generator rather than merely new numbers. Mirror Pass and Thieves Gold are built on a reflection instead of a rotation: the halves face each other rather than being turned around the middle, so anything on the axis, the gem pit, the lake, serves both players at the same distance instead of coming in pairs. And the lake itself is a new centre: a gold island ringed by water, which stops a digging worker in a way no wall of dirt does, so the only ways in are the authored causeways. Every causeway is authored in the full orbit, since each one crosses water the other stamps would re-lay. Old Enemies is the first one on one skirmish with a hero vault, and The Last Gate spreads six portals across the widest survival front so far. Sizes 89 to 137 fill the gaps between the existing maps; all sizes stay odd so the symmetry has a centre tile to fix. Checked the same way as the other nineteen: the grid maps onto itself under the declared transform with the seat permutation derived from where the temples land, early reachable gold is equal for every player, and every temple digs to every portal and temple without crossing water or lava. The nineteen existing maps regenerate byte for byte with the new code. All eight load in the engine and hold a listening server with no warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The spells tab put its ten buttons in a single row of 60 pixel squares running out to x=700. The pane they live in is the window width minus the 200 pixels of minimap, so at the 800 wide minimum window it is 600 pixels across, and the last two spells, Weakness and the Eye of Evil, hung past its right edge where they could be neither seen nor clicked. Only a window at least 900 wide showed the whole row, and nothing said so. The rooms tab solved the same problem long ago: two rows of 40 pixel buttons. The spells tab now uses the same shape, five spells to a row, ending at x=300 with room to spare at any size the game accepts. The cooldown bar inside each button now covers exactly its button too, instead of overhanging 20 pixels on both sides, which with narrower buttons would have bled half way across the neighbours. The rooms tab had the tail of the same bug: the temple and portal buttons the editor uses sat at x=570 to 690, past the same edge. They join the end of the second row instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
|
sorry for this huge PR... I made a test with Claude Opus to see if it can fix and expand this game. Each commit has an extensive description. |
The waves of a wave portal could only be written by hand in the level file. They are now edited from the editor: point at a portal, press P, and a window shows the turns between waves, the strategy, the attack range and the target teams, along with the list of waves and the creatures of each of them. A client is never told about the rooms of a map, only about what its tiles look like, and the waves are not part of what a room sends anyway. So the editor names the portal to the server by one of its tiles and asks for the waves; the server answers with them, and sends back what the player applied. The level is saved from the server side, so that is where the change has to land. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every seat writes the tiles it has ever seen, and the loop over the columns of the map was bounded by the height of the map instead of its width. On a map taller than it is wide it read past the end of the array: DuelToDeath is 120 by 151, and saving a game there left a level file cut off in the middle of that list and took the game down with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
@Upabjojr hello, great to have a new contributor, do you mind joining our Discord channel and introduce yourself ? The link is the game when you close the application... |
Owner
|
BTW: can you help in similar manner with this one : #15 it almost works , except the CEGUI , go to OPTIONS-> AUDIO or GAMEPLAY. The percentage slider would be gone ( that's a tiny bitmap resembling an X sign -- at it gets lost in transition... donno why ) |
Author
I tried it... it's generated a lot of stuff in the other PR. I tried Claude Fable 5 this time instead of Claude Opus. |
Owner
|
IT might take time before I read everything ... so far I have checked out what is here marked as "Crash fixes " .... do you mind me I cherrypick the commits from your branch one at a time ? |
Author
|
Yes, no problem with cherry picking. |
This was referenced Aug 1, 2026
Author
|
This PR has been split into 14 focused PRs, one per separable topic, to make review manageable:
Each branch starts from this repo's 🤖 Generated with Claude Code |
Owner
|
great , but ... I have a feeling that not all commits were preserved :\ Fix the editor re-entry crash pair: CEGUI AlreadyExistsException from menus being filled twice (8ec6c20) and a segfault from EditorMode deleting the render manager's DebugDrawer singleton (893df06) I cannot find it in any of the new PR's ! |
Author
Probably Claude was either unable to separate them or it made a mistake. |
Owner
|
Can you introduce yourself to other developers here : https://discord.gg/K2JPXuchZV please please :) |
Author
|
All 26 commits of this PR were carried over into the split PRs — none were dropped. What changed is their hashes: the commits were cherry-picked onto this repo's The two named in the review feedback:
Full mapping, old → new:
(#30's three commits were additionally reworked afterwards — richer terrain, wildlife and hero dungeons — as described in that PR; the other PRs carry the original changes unmodified apart from the three trivial context-conflict resolutions noted in the PR descriptions.) 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch collects the work done over the last sessions: crash fixes, gameplay/UI fixes, editor quality-of-life features, cross-platform (Windows/macOS) portability work, and 27 new skirmish/multiplayer/survival levels.
Crash fixes
AlreadyExistsExceptionfrom menus being filled twice (8ec6c20) and a segfault fromEditorModedeleting the render manager'sDebugDrawersingleton (893df06)Seat's per-tile state lookups: two end-iterator dereferences, a dead fallback and four accidental map inserts, replaced by a single checked helper (3804e4a)Gameplay and UI fixes
Editor improvements
Cross-platform work
SetClassLongPtr, open URLs with the native mechanism per platform, and strip CRLF line endings in the single file-reading funnel so Windows-edited level/config files load everywhere (db2e230, f45b427)New levels
Tests
Note: the GUI changes (two-row spells tab, editor help window) were verified by XML validity and code-path inspection but not rendered, as this was developed on a headless setup. The Windows/macOS fixes are compile-checked where reachable from Linux.
🤖 Generated with Claude Code