Remaining fixes from the PR 15 review discussion - #2
Merged
slapin merged 10 commits intoAug 14, 2026
Conversation
materials/RTShaderLib/GLSL appears in no resources.cfg group and Ogre 14 ships both files itself at @RTSHADER_DIR@ (Media/RTShaderLib), which the Graphics group already lists. The in-tree copies were installed with the rest of materials/ but never reachable by the resource system, and they would silently drift from whatever Ogre version is actually installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ephemeral-port fallback in createServer() made single player robust, but it also applied while hosting: if a LAN host's configured port was busy the server silently bound another port, remote clients kept dialing the advertised one, and the host got a connection that never establishes instead of an error. Local modes (skirmish, editor, loaded saves) still fall back — their client reaches the server through getNetworkPort(), which reports the port actually bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RTShaderSystem.material references Panels_Diffuse.png in both RTSS/PerPixel_SinglePass (the base of Bed.material and Cannonball.material) and RTSS/NormalMapping_MultiPass, but the texture only ships in Ogre's sample media, which stock Ogre installs leave out. Panels_Normal_Tangent.png was already vendored for the same reason — this adds its diffuse companion, byte-for-byte from OGRECave/ogre Samples/Media/materials/textures (MIT), and credits both in CREDITS. Also drops the CREDITS entry for the RTShaderLib copies deleted earlier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The one-shot scaling in the constructor pinned the settings and apply-changes windows to pixel offsets computed for the resolution at construction time — and this is the very window resolutions are applied from, so after a resolution change it stayed sized for the old one until restart. Scaling is now redone on System::EventDisplaySizeChanged (ODFrameListener::windowResized already fires it): child widgets move between scales by plain offset ratio, and the top-level window is re-centered from its designed area each time. This also fixes a mixed-scale bug: initConfig() recreates the extra video-option widgets after applying settings, and they were laid out in unscaled design pixels inside an already-scaled window. They are now brought to the current UI scale on creation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The port gave OD/FrameWindow a proper ClientWithTitleWithFrame area (it used to be an empty <Area/>, i.e. the whole window). Correct, but the quit, load-confirm and apply-changes dialogs were laid out against the old full-window coordinates, so in the shrunken client area their buttons landed on top of the text and bottom widgets (the save-replay checkbox) fell below the clipped edge. Anchor the button rows to the client area's bottom edge and let the text own the space above them, so the dialogs lay out correctly whatever room the titlebar and frame take. Also fixes ApplyText's VertFormatting property, which was spelled with capitalized XML attributes (Name=/ Value=) and therefore silently ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The rename from 'invocation' to 'cameraName' is a semantic claim about what Ogre passes here, and it silently decides whether CEGUI ever gets drawn. Name the Ogre version and upstream source so the next reader does not have to re-do the archaeology. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The creature stats text has ~22 lines nowadays and the frame's client area no longer includes the titlebar, so the tail of the text (the mood lines) was clipped at the frame's bottom edge. Give the text pane the whole client area, enable the vertical scrollbar OD/StaticText already supports, and make the creature window a bit taller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The explanatory comment and the FIXME left from debugging contradicted each other. State plainly why the X11 cursor is hidden unconditionally (CEGUI draws its own cursor; with grabbing, OIS warping makes the system cursor drift) and what the trade-off is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Runtime-verified against Ogre 14.6 + the CEGUI fork: the OD/FrameWindow titlebar is ~45px tall, so the 139px quit dialog left only ~55px of client area — not enough for the button row plus the replay checkbox, and the buttons overlapped the title text. Make the quit and apply-changes dialogs 190px tall; with the bottom-anchored rows the dialogs now render with the full question in the titlebar, the buttons in a clean row and the checkbox below. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Oki those are fine, except the titlebar is still misplaced, other parts of the Quit Menu Window are fine : |
|
The titlebar (and close button) auto-widgets were positioned relative to the frame's client area — but the client area starts below the titlebar, so the titlebar rendered its own height too low, leaving a strip of bare background between it and the window's top border and wasting the same amount of client space. That is the 'titlebar is misplaced' from the PR discussion. Mark both auto-widgets NonClient so they resolve against the window's outer rect: the titlebar sits at the top edge, stably, and the client area computed from its bottom edge is correct. Verified in-game against Ogre 14.6 + the CEGUI fork on the settings window and the apply-changes dialog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
Thanks for testing! Two separate things in your screenshot: The buttons sitting on the title text — your build predates the branch's newest commits: the dialog in your screenshot is still ~139px tall, and The titlebar itself being misplaced — that one was real, and deeper than the dialogs: the Verified in-game against Ogre 14.6 + the CEGUI fork — titlebar now hugs the top border, and the dialogs lay out cleanly:
Unrelated to this PR but worth knowing for your CEGUI build: the "slider renders nothing with AutoRenderingSurface" bug in the fork's renderer is now root-caused and fixed too — slapin/cegui#1. 🤖 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
3 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.
Follow-up to #1, covering what still stood from the tomluchowski#15 discussion after the rebase. Six commits, each self-contained:
materials/RTShaderLib/GLSL/{RTSLib_Colour,SGXLib_NormalMap}.glslare in no resource group and Ogre 14 ships both at@RTSHADER_DIR@, which[Graphics]already lists.getNetworkPort(). A LAN host with a busy configured port fails loudly again instead of silently binding a port no remote client will dial.Panels_Diffuse.png—RTShaderSystem.materialreferences it from bothRTSS/PerPixel_SinglePass(base ofBed/Cannonball) andNormalMapping_MultiPass, but it only ships in Ogre's sample media. Byte-for-byte from OGRECave/ogreSamples/Media/materials/textures(MIT), same provenance as the already-vendoredPanels_Normal_Tangent.png; both now credited in CREDITS.System::EventDisplaySizeChanged(whichwindowResizedalready fires). Also fixes a latent mixed-scale bug: the video-option widgetsinitConfig()recreates after applying settings were laid out unscaled inside a scaled window.ClientWithTitleWithFramefix inOD.looknfeel(previously an empty<Area/>, i.e. the whole window) is correct, butWindowQuit,LoadConfirmandWindowApplyChangeswere laid out against the old full-window coordinates, so their buttons landed on the text and the save-replay checkbox fell below the clipped bottom edge. Buttons are now anchored to the client area's bottom, text owns the space above, andApplyText'sVertFormattingproperty is spelled with lowercasename=/value=so it stops being silently ignored.renderQueueStarted— names the Ogre version thecameraNameparameter semantics come from, as discussed in review.Verification: no Ogre 14 available on this machine this time, so not compile-tested against the real target; the changed C++ files pass
g++ -fsyntax-onlyagainst CEGUI 0.8.7 headers (only version-stable APIs are used —System::EventDisplaySizeChangedexists in the fork'sSystem.h:81), and the layouts pass xmllint. The dialog re-anchoring deserves a visual check.🤖 Generated with Claude Code