-
Notifications
You must be signed in to change notification settings - Fork 100
Radius overlay #1946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Radius overlay #1946
Changes from 16 commits
fa453a0
3175b05
d489b4c
5728636
d6c53b8
11df35a
0572598
cfebdce
795a110
bdfd9b1
7957d83
1f70192
4f4fa6e
7ec4ce5
ab91acd
16840a8
0ed0c34
e4df373
675e474
0f408f0
caef896
ae81945
93246c4
83752d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||
| // Copyright (C) 2005 - 2026 Settlers Freaks (sf-team at siedler25.org) | ||||||||
| // | ||||||||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||||||||
|
|
||||||||
| #pragma once | ||||||||
|
|
||||||||
| #include "AddonBool.h" | ||||||||
| #include "mygettext/mygettext.h" | ||||||||
|
|
||||||||
| /** | ||||||||
| * Show building radius information in tooltips and as map overlay | ||||||||
| */ | ||||||||
| class AddonBuildingRadius : public AddonBool | ||||||||
| { | ||||||||
| public: | ||||||||
| AddonBuildingRadius() | ||||||||
| : AddonBool(AddonId::BUILDING_RADIUS, AddonGroup::GamePlay, _("Show building radius"), | ||||||||
| _("Shows the working radius of buildings in the build menu tooltip and as an overlay on the map " | ||||||||
| "when hovering over a building icon or selecting a building."), | ||||||||
| 1) // Enabled by default | ||||||||
|
Comment on lines
+19
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The convention is that the default is S2-like behavior, so you can "disable" all addons and get vanilla S2
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah can revert this. Making it default enabled allowed me to continue using my save game for testing tho so it's been nice. Also playing campaign with it enabled is lovely. I'm addicted I can't play without it. Let's revert to default disable last thing before merge ok? |
||||||||
| {} | ||||||||
| }; | ||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,13 @@ | |||||
| // SPDX-License-Identifier: GPL-2.0-or-later | ||||||
|
|
||||||
| #include "BuildingConsts.h" | ||||||
| #include "figures/nofCatapultMan.h" | ||||||
| #include "figures/nofFarmhand.h" | ||||||
| #include "figures/nofHunter.h" | ||||||
| #include "mygettext/mygettext.h" | ||||||
| #include "gameTypes/BuildingTypes.h" | ||||||
| #include "gameData/GameConsts.h" | ||||||
| #include "gameData/MilitaryConsts.h" | ||||||
| #include <type_traits> | ||||||
|
|
||||||
| const helpers::EnumArray<const char*, BuildingType> BUILDING_NAMES = { | ||||||
|
|
@@ -409,3 +415,48 @@ const helpers::MultiEnumArray<DrawPoint, Nation, BuildingType> BUILDING_ARMOR_SI | |||||
| babylonians[BuildingType::Fortress] = DrawPoint(20, -34); | ||||||
| return result; | ||||||
| }(); | ||||||
|
|
||||||
| unsigned GetBuildingRadius(BuildingType bld) | ||||||
| { | ||||||
| switch(bld) | ||||||
| { | ||||||
| // Military buildings (territory influence radius) — from MilitaryConsts.h | ||||||
| case BuildingType::Barracks: return MILITARY_RADIUS[0]; | ||||||
| case BuildingType::Guardhouse: return MILITARY_RADIUS[1]; | ||||||
| case BuildingType::Watchtower: return MILITARY_RADIUS[2]; | ||||||
| case BuildingType::Fortress: return MILITARY_RADIUS[3]; | ||||||
| // Headquarters | ||||||
| case BuildingType::Headquarters: return HQ_RADIUS; | ||||||
| // Harbor building | ||||||
| case BuildingType::HarborBuilding: return HARBOR_RADIUS; | ||||||
| // Lookout tower — scouting visibility range | ||||||
| case BuildingType::LookoutTower: return VISUALRANGE_LOOKOUTTOWER; | ||||||
| // Catapult attack range | ||||||
| case BuildingType::Catapult: return CATAPULT_MAX_TARGET_RANGE; | ||||||
| // Hunter searches for animals in a square of this half-side length | ||||||
| case BuildingType::Hunter: return HUNTER_SEARCH_HALFSIDE; | ||||||
| // Mines — miner stays inside and extracts from adjacent tiles | ||||||
| case BuildingType::GraniteMine: | ||||||
| case BuildingType::CoalMine: | ||||||
| case BuildingType::IronMine: | ||||||
| case BuildingType::GoldMine: return MINER_RADIUS; | ||||||
| // Farmhand-based buildings — worker goes out to gather resources from the map. | ||||||
| // Map each building type to its job via BLD_WORK_DESC, then query the work | ||||||
| // radius from nofFarmhand::GetWorkRadius. | ||||||
| case BuildingType::Woodcutter: | ||||||
| case BuildingType::Forester: | ||||||
| case BuildingType::Fishery: | ||||||
| case BuildingType::Quarry: | ||||||
| case BuildingType::Farm: | ||||||
| case BuildingType::Vineyard: | ||||||
| case BuildingType::Charburner: | ||||||
| { | ||||||
| const auto job = BLD_WORK_DESC[bld].job; | ||||||
| if(job) | ||||||
| return nofFarmhand::GetWorkRadius(*job); | ||||||
| return 0; | ||||||
| } | ||||||
| // All other building types have no relevant radius overlay | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| default: return 0; | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |||||||||
| #include "nodeObjs/noFlag.h" | ||||||||||
| #include "gameData/BuildingConsts.h" | ||||||||||
| #include "gameData/const_gui_ids.h" | ||||||||||
| #include <boost/format.hpp> | ||||||||||
| #include <sstream> | ||||||||||
|
|
||||||||||
| // Tab - Flags | ||||||||||
|
|
@@ -159,6 +160,7 @@ iwAction::iwAction(GameInterface& gi, GameWorldView& gwv, const Tabs& tabs, MapP | |||||||||
| building_available[BuildingType::LeatherWorks] = false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const bool showBuildingRadius = gwv.GetWorld().GetGGS().isEnabled(AddonId::BUILDING_RADIUS); | ||||||||||
| constexpr helpers::EnumArray<unsigned, BuildTab> NUM_TABS = {1, 2, 3, 1, 3}; | ||||||||||
|
|
||||||||||
| for(unsigned char i = 0; i < NUM_TABS[tabs.build_tabs]; ++i) | ||||||||||
|
|
@@ -175,6 +177,13 @@ iwAction::iwAction(GameInterface& gi, GameWorldView& gwv, const Tabs& tabs, MapP | |||||||||
| std::stringstream tooltip; | ||||||||||
| tooltip << _(BUILDING_NAMES[bld]); | ||||||||||
|
|
||||||||||
| // Radius anzeigen falls vorhanden | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. English comments only (at least for new/changed ones) |
||||||||||
| unsigned radius = 0; | ||||||||||
| if(showBuildingRadius) | ||||||||||
| radius = GetBuildingRadius(bld); | ||||||||||
|
Comment on lines
+181
to
+183
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer const variables:
Suggested change
|
||||||||||
| if(radius > 0) | ||||||||||
| tooltip << boost::format(_("\nRange: %1% tiles")) % radius; | ||||||||||
|
|
||||||||||
| tooltip << _("\nCosts: "); | ||||||||||
| if(BUILDING_COSTS[bld].boards > 0) | ||||||||||
| tooltip << (int)BUILDING_COSTS[bld].boards << _(" boards"); | ||||||||||
|
|
@@ -407,6 +416,7 @@ void iwAction::Close() | |||||||||
| { | ||||||||||
| if(ShouldBeClosed()) | ||||||||||
| return; | ||||||||||
| gwv.SetRadiusPreview(std::nullopt); | ||||||||||
| IngameWindow::Close(); | ||||||||||
| if(mousePosAtOpen_.isValid()) | ||||||||||
| VIDEODRIVER.SetMousePos(mousePosAtOpen_); | ||||||||||
|
|
@@ -526,6 +536,30 @@ void iwAction::Msg_Group_TabChange(const unsigned /*group_id*/, const unsigned c | |||||||||
| void iwAction::Msg_PaintAfter() | ||||||||||
| { | ||||||||||
| IngameWindow::Msg_PaintAfter(); | ||||||||||
|
|
||||||||||
| // Resolve building icon hover preview after all mouse events are processed | ||||||||||
| auto* mainTab = GetCtrl<ctrlTab>(0); | ||||||||||
| auto* buildTabCtrl = | ||||||||||
| (mainTab && mainTab->GetCurrentTab() == TAB_BUILD) ? mainTab->GetGroup(TAB_BUILD)->GetCtrl<ctrlTab>(1) : nullptr; | ||||||||||
| auto* bldGroup = buildTabCtrl ? buildTabCtrl->GetGroup(buildTabCtrl->GetCurrentTab()) : nullptr; | ||||||||||
|
|
||||||||||
| bool hasHoveredIcon = false; | ||||||||||
| if(bldGroup && gwv.GetWorld().GetGGS().isEnabled(AddonId::BUILDING_RADIUS)) | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks inefficient doing on every single draw call. What was wrong with the MouseMove-hovering?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an issue with icon->SetOnHoverChanged() Maybe there should be 2 passes, first send leave event, then send enter. |
||||||||||
| { | ||||||||||
| for(auto* icon : bldGroup->GetCtrls<ctrlBuildingIcon>()) | ||||||||||
| { | ||||||||||
| const unsigned radius = GetBuildingRadius(icon->GetType()); | ||||||||||
| if(icon->IsMouseOver() && radius > 0) | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't you break once you found ANY mouseover? |
||||||||||
| { | ||||||||||
| gwv.SetRadiusPreview(std::make_pair(selectedPt, radius)); | ||||||||||
| hasHoveredIcon = true; | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| if(!hasHoveredIcon) | ||||||||||
| gwv.SetRadiusPreview(std::nullopt); | ||||||||||
|
|
||||||||||
| auto* tab = GetCtrl<ctrlTab>(0); | ||||||||||
| if(tab) | ||||||||||
| { | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,10 @@ | |||||||
| #include "GlobalGameSettings.h" | ||||||||
| #include "Loader.h" | ||||||||
| #include "MapGeometry.h" | ||||||||
| #include "ReturnMapPointWithRadius.h" | ||||||||
| #include "Settings.h" | ||||||||
| #include "Window.h" | ||||||||
| #include "WindowManager.h" | ||||||||
| #include "addons/AddonMaxWaterwayLength.h" | ||||||||
| #include "buildings/noBuildingSite.h" | ||||||||
| #include "buildings/nobMilitary.h" | ||||||||
|
|
@@ -36,6 +39,7 @@ | |||||||
| #include <glad/glad.h> | ||||||||
| #include <boost/format.hpp> | ||||||||
| #include <cmath> | ||||||||
| #include <optional> | ||||||||
|
|
||||||||
| GameWorldView::GameWorldView(const GameWorldViewer& gwv, const Position& pos, const Extent& size) | ||||||||
| : selPt(0, 0), show_bq(SETTINGS.ingame.showBQ), show_names(SETTINGS.ingame.showNames), | ||||||||
|
|
@@ -220,6 +224,37 @@ void GameWorldView::Draw(const RoadBuildState& rb, const MapPoint selected, bool | |||||||
| if(show_names || show_productivity) | ||||||||
| DrawNameProductivityOverlay(terrainRenderer); | ||||||||
|
|
||||||||
| // Draw radius preview outline | ||||||||
| if(radiusPreview_) | ||||||||
| DrawRadiusOutline(radiusPreview_->first, radiusPreview_->second); | ||||||||
|
|
||||||||
| // Auto-detect radius for the building under the cursor. | ||||||||
| // Do not trigger hover when the mouse is over an ingame window. | ||||||||
| if(!radiusPreview_ && GetWorld().GetGGS().isEnabled(AddonId::BUILDING_RADIUS) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't that be rather on the outside, e.g. in dskGameInterface where it could be more efficiently in a MouseMove handler. This way it is opt-in and e.g. doesn't enable for observation windows (suspicion only, haven't checked) |
||||||||
| && !WINDOWMANAGER.FindWindowAtPos(VIDEODRIVER.GetMousePos())) | ||||||||
| { | ||||||||
| std::optional<BuildingType> bldType; | ||||||||
| const Visibility vis = gwv.GetVisibility(selPt); | ||||||||
| if(vis == Visibility::Visible) | ||||||||
| { | ||||||||
| const auto* bld = GetWorld().GetSpecObj<noBaseBuilding>(selPt); | ||||||||
| if(bld) | ||||||||
| bldType = bld->GetBuildingType(); | ||||||||
| } else if(vis == Visibility::FogOfWar) | ||||||||
| { | ||||||||
| const FOWObject* fow = gwv.GetYoungestFOWObject(selPt); | ||||||||
| if(fow && fow->GetType() == FoW_Type::Building) | ||||||||
| bldType = static_cast<const fowBuilding&>(*fow).GetBuildingType(); | ||||||||
| } | ||||||||
|
|
||||||||
| if(bldType) | ||||||||
| { | ||||||||
| const unsigned bldRadius = GetBuildingRadius(*bldType); | ||||||||
| if(bldRadius > 0) | ||||||||
| DrawRadiusOutline(selPt, bldRadius); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| DrawGUI(rb, terrainRenderer, selected, drawMouse); | ||||||||
|
|
||||||||
| // Draw catapult stones | ||||||||
|
|
@@ -713,6 +748,60 @@ void GameWorldView::RemoveDrawNodeCallback(IDrawNodeCallback* callbackToRemove) | |||||||
| drawNodeCallbacks.erase(itPos); | ||||||||
| } | ||||||||
|
|
||||||||
| // ----------------------------------------------------------------------------- | ||||||||
| // Snap a point to the nearest toroidal copy relative to a reference position. | ||||||||
| // Same formula as s25edit's correctMouseBlit(): | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't include such references. The client should be self-contained and the code correct on its own. So rather explain why this specific code is correct as-is. |
||||||||
| // k = round((reference - vertex) / mapSize) | ||||||||
| // vertex += k * mapSize | ||||||||
| // | ||||||||
| // Reference: s25edit/external/s25edit/CMap.cpp :: correctMouseBlit() (line 1088) | ||||||||
| // ----------------------------------------------------------------------------- | ||||||||
| DrawPoint GameWorldView::SnapToNearestCopy(DrawPoint pt, const DrawPoint& ref, const DrawPoint& mapPxSize) | ||||||||
| { | ||||||||
| if(mapPxSize.x > 0) | ||||||||
| { | ||||||||
| int kx = static_cast<int>(std::floor((ref.x - pt.x) / static_cast<double>(mapPxSize.x) + 0.5)); | ||||||||
| pt.x += kx * mapPxSize.x; | ||||||||
| } | ||||||||
| if(mapPxSize.y > 0) | ||||||||
| { | ||||||||
| int ky = static_cast<int>(std::floor((ref.y - pt.y) / static_cast<double>(mapPxSize.y) + 0.5)); | ||||||||
| pt.y += ky * mapPxSize.y; | ||||||||
| } | ||||||||
| return pt; | ||||||||
| } | ||||||||
|
|
||||||||
| // ----------------------------------------------------------------------------- | ||||||||
| // Draw radius overlay using CorrectMouseBlit for toroidal wrapping. | ||||||||
| // Reference: s25edit/external/s25edit/CMap.cpp :: render() (lines 1159, 1230) | ||||||||
| // - Computes brush blit positions via correctMouseBlit() | ||||||||
| // - Draws overlay sprites at those positions | ||||||||
| // ----------------------------------------------------------------------------- | ||||||||
| void GameWorldView::DrawRadiusOutline(const MapPoint& center, unsigned radius) | ||||||||
| { | ||||||||
| const auto& world = GetWorld(); | ||||||||
| auto pts = world.GetPointsInRadius(center, radius, ReturnMapPointWithRadius{}); | ||||||||
|
|
||||||||
| const MapExtent mapSize = world.GetSize(); | ||||||||
| constexpr unsigned BORDER_COLOR = 0xFFFF0000; | ||||||||
| const DrawPoint mapPxSize(mapSize.x * TR_W, mapSize.y * TR_H); | ||||||||
|
|
||||||||
| // Reference: screen position of the center vertex (with seam offset). | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why "reference"? Isn't this simply a position in screen coordinates? I.e. a |
||||||||
| const auto centerAlt = world.GetNode(center).altitude; | ||||||||
| const DrawPoint ref = GetNodePos(center) - DrawPoint(0, HEIGHT_FACTOR * centerAlt) + selPtOffset; | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the overload:
Suggested change
Also where does |
||||||||
|
|
||||||||
| for(const auto& [basePt, dist] : pts) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Or |
||||||||
| { | ||||||||
| if(dist != radius) | ||||||||
| continue; | ||||||||
|
|
||||||||
| DrawPoint scr = GetNodePos(basePt, world.GetNode(basePt).altitude); | ||||||||
| scr = SnapToNearestCopy(scr, ref, mapPxSize) - offset; | ||||||||
|
|
||||||||
| Window::DrawRectangle(Rect(scr - DrawPoint(2, 2), Extent(5, 5)), BORDER_COLOR); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| void GameWorldView::CalcFxLx() | ||||||||
| { | ||||||||
| // Calc first and last point in map units (with 1 extra for incomplete triangles) | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem correct. Please revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to allow default enabled addons.
Ok I find it very useful to have it default enabled for testing. Can we revert this last thing before merge.