From 68adab93b3e961d14fb0dbec282f18d5bb18570a Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 19:46:24 -0700 Subject: [PATCH 01/13] Add Africa and Port Royal as trade route export destinations Extends the trade route system to support Africa and Port Royal as off-map trade locations alongside Europe. Adds sentinel IDs, the isOffMapTradeLocation() helper, route validation, AI evaluation, Python UI support in the Trade Routes Advisor, and canSailToAfrica/ canSailToPortRoyal Python bindings. Co-Authored-By: Claude Opus 4.6 --- Assets/Python/Screens/CvTradeRoutesAdvisor.py | 65 +++++++++++++++---- Project Files/DLLSources/CvCity.cpp | 16 +++++ Project Files/DLLSources/CvDLLButtonPopup.cpp | 11 ++-- Project Files/DLLSources/CvPlayer.cpp | 62 ++++++++++++------ Project Files/DLLSources/CvPlayerAI.cpp | 3 +- .../DLLSources/CvSelectionGroupAI.cpp | 24 ++++--- Project Files/DLLSources/CvTradeRoute.cpp | 34 +++++++++- Project Files/DLLSources/CvTradeRoute.h | 7 ++ Project Files/DLLSources/CvTradeRouteGroup.h | 3 - Project Files/DLLSources/CvUnit.cpp | 29 +++++++-- Project Files/DLLSources/CyUnit.cpp | 8 +++ Project Files/DLLSources/CyUnit.h | 2 + Project Files/DLLSources/CyUnitInterface2.cpp | 2 + 13 files changed, 204 insertions(+), 62 deletions(-) diff --git a/Assets/Python/Screens/CvTradeRoutesAdvisor.py b/Assets/Python/Screens/CvTradeRoutesAdvisor.py index 7c2f04640..187fe482b 100644 --- a/Assets/Python/Screens/CvTradeRoutesAdvisor.py +++ b/Assets/Python/Screens/CvTradeRoutesAdvisor.py @@ -55,6 +55,9 @@ def __init__(self): self.NO_YIELD = -1 self.EUROPE_CITY = -1 self.NO_CITY = -2 + self.AFRICA_CITY = -3 + self.PORT_ROYAL_CITY = -4 + self.OFF_MAP_CITIES = [self.EUROPE_CITY, self.AFRICA_CITY, self.PORT_ROYAL_CITY] # Button ids self.YIELD_TABLE_ID = 0 @@ -79,6 +82,21 @@ def __init__(self): #R&R mod, vetiarvind, trade groups - END + def isOffMapCity(self, iCityId): + return iCityId in self.OFF_MAP_CITIES + + def isRealCity(self, iCityId): + return iCityId != self.NO_CITY and not self.isOffMapCity(iCityId) + + def getOffMapCityName(self, iCityId): + if iCityId == self.EUROPE_CITY: + return localText.getText("TXT_KEY_CONCEPT_EUROPE", ()) + elif iCityId == self.AFRICA_CITY: + return localText.getText("TXT_KEY_CONCEPT_AFRICA", ()) + elif iCityId == self.PORT_ROYAL_CITY: + return localText.getText("TXT_KEY_CONCEPT_PORT_ROYAL", ()) + return u"" + def interfaceScreen (self): screen = self.getScreen() if screen.isActive(): @@ -346,9 +364,9 @@ def sortByDestinationCities(pRoute): def getColor(self, pRoute): szColor = u"" - if pRoute.getDestinationCity().iID == self.EUROPE_CITY: + if self.isOffMapCity(pRoute.getDestinationCity().iID): szColor = u"" - + return szColor def updateRoutes(self): @@ -384,10 +402,10 @@ def appendBuilderRow(self): if self.iExport != self.NO_CITY: szExport = u"%s" % self.player.getCity(self.iExport).getName() szImport = localText.getText("TXT_KEY_TRADE_ROUTES_MISSING_CITY", ()) - if self.iImport > self.EUROPE_CITY: + if self.isRealCity(self.iImport): szImport = u"%s" % self.player.getCity(self.iImport).getName() - elif self.iImport == self.EUROPE_CITY: - szImport = localText.getText("TXT_KEY_CONCEPT_EUROPE", ()) + elif self.isOffMapCity(self.iImport): + szImport = self.getOffMapCityName(self.iImport) szTable = self.TableNames[self.CURRENT_TABLE] szColor = u"" @@ -493,13 +511,34 @@ def cityTable(self, bImport): #Europe screen.appendTableRow(szTable) screen.setTableRowHeight(szTable, iI, self.ROW_HIGHT) - screen.setTableText(szTable, 0, iI, u"-1", "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 0, iI, u"%d" % self.EUROPE_CITY, "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) screen.setTableText(szTable, 1, iI, u"%c" % CyGame().getSymbolID(FontSymbols.ANCHOR_EUROPE_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) screen.setTableText(szTable, 2, iI, localText.getText("TXT_KEY_CONCEPT_EUROPE", ()), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_LEFT_JUSTIFY) screen.setTableText(szTable, 3, iI, u"-", "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) screen.setTableText(szTable, 4, iI, u"%c" % CyGame().getSymbolID(FontSymbols.IMPORT_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) - iI += 1 + + #Africa + if self.pTransport.canSailToAfrica(): + screen.appendTableRow(szTable) + screen.setTableRowHeight(szTable, iI, self.ROW_HIGHT) + screen.setTableText(szTable, 0, iI, u"%d" % self.AFRICA_CITY, "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 1, iI, u"%c" % CyGame().getSymbolID(FontSymbols.ANCHOR_EUROPE_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 2, iI, localText.getText("TXT_KEY_CONCEPT_AFRICA", ()), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_LEFT_JUSTIFY) + screen.setTableText(szTable, 3, iI, u"-", "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 4, iI, u"%c" % CyGame().getSymbolID(FontSymbols.IMPORT_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + iI += 1 + + #Port Royal + if self.pTransport.canSailToPortRoyal(): + screen.appendTableRow(szTable) + screen.setTableRowHeight(szTable, iI, self.ROW_HIGHT) + screen.setTableText(szTable, 0, iI, u"%d" % self.PORT_ROYAL_CITY, "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 1, iI, u"%c" % CyGame().getSymbolID(FontSymbols.ANCHOR_EUROPE_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 2, iI, localText.getText("TXT_KEY_CONCEPT_PORT_ROYAL", ()), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_LEFT_JUSTIFY) + screen.setTableText(szTable, 3, iI, u"-", "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + screen.setTableText(szTable, 4, iI, u"%c" % CyGame().getSymbolID(FontSymbols.IMPORT_CHAR), "", WidgetTypes.WIDGET_GENERAL, -1, -1, CvUtil.FONT_CENTER_JUSTIFY) + iI += 1 for city in self.CityList: screen.appendTableRow(szTable) @@ -723,7 +762,7 @@ def updatePreview(self): CURRENT_X += self.PREVIEW_WIDTH + self.STANDARD_MARGIN / 2 - if self.iImport > self.EUROPE_CITY: + if self.isRealCity(self.iImport): if self.iImport != self.iImportPreview: self.iImportPreview = self.iImport screen.show(self.szPreviewImport + "Banner") @@ -735,11 +774,13 @@ def updatePreview(self): screen.setLabelAt(self.szPreviewImport + "Label", self.szPreviewImport + "Banner", u"" + u"%s" % self.player.getCity(self.iImport).getName() + u"", CvUtil.FONT_CENTER_JUSTIFY, (self.PREVIEW_WIDTH - 20) / 2, 10, 0, FontTypes.GAME_FONT, WidgetTypes.WIDGET_GENERAL, -1, -1) screen.addPlotGraphicGFC(self.szPreviewImport, CURRENT_X + 6, self.PREVIEW_Y + 6, self.PREVIEW_WIDTH - 12, self.PREVIEW_HEIGHT - 12, self.player.getCity(self.iImport).plot(), 350, True, WidgetTypes.WIDGET_GENERAL, -1, -1) screen.moveToFront(self.szPreviewImport + "Border" + str(0)) - elif self.iImport == self.EUROPE_CITY: + elif self.isOffMapCity(self.iImport): screen.show(self.szPreviewImport + "Banner") screen.show(self.szPreviewImport + "Cancel") - screen.setLabelAt(self.szPreviewImport + "Label", self.szPreviewImport + "Banner", u"" + localText.getText("TXT_KEY_CONCEPT_EUROPE", ()) + u"", CvUtil.FONT_CENTER_JUSTIFY, (self.PREVIEW_WIDTH - 10) / 2, 10, 0, FontTypes.GAME_FONT, WidgetTypes.WIDGET_GENERAL, -1, -1) - screen.addDDSGFC(self.szPreviewImport, "Art/Interface/Screens/TradeRoutes/EuropePreview.dds", CURRENT_X + 6, self.PREVIEW_Y + 6, self.PREVIEW_WIDTH - 12, self.PREVIEW_HEIGHT - 12, WidgetTypes.WIDGET_GENERAL, -1, -1 ) + szLocationName = self.getOffMapCityName(self.iImport) + szPreviewArt = "Art/Interface/Screens/TradeRoutes/EuropePreview.dds" + screen.setLabelAt(self.szPreviewImport + "Label", self.szPreviewImport + "Banner", u"" + szLocationName + u"", CvUtil.FONT_CENTER_JUSTIFY, (self.PREVIEW_WIDTH - 10) / 2, 10, 0, FontTypes.GAME_FONT, WidgetTypes.WIDGET_GENERAL, -1, -1) + screen.addDDSGFC(self.szPreviewImport, szPreviewArt, CURRENT_X + 6, self.PREVIEW_Y + 6, self.PREVIEW_WIDTH - 12, self.PREVIEW_HEIGHT - 12, WidgetTypes.WIDGET_GENERAL, -1, -1 ) screen.moveToFront(self.szPreviewImport + "Border" + str(0)) else: screen.hide(self.szPreviewImport) @@ -797,7 +838,7 @@ def addSelection(self): return CyMessageControl().sendDoTask(self.iExport, TaskTypes.TASK_YIELD_EXPORT, self.iYields, True, False, False, False, False) - if self.iImport > self.EUROPE_CITY: + if self.isRealCity(self.iImport): CyMessageControl().sendDoTask(self.iImport, TaskTypes.TASK_YIELD_IMPORT, self.iYields, True, False, False, False, False) diff --git a/Project Files/DLLSources/CvCity.cpp b/Project Files/DLLSources/CvCity.cpp index 991796ed2..6eff190de 100644 --- a/Project Files/DLLSources/CvCity.cpp +++ b/Project Files/DLLSources/CvCity.cpp @@ -12234,6 +12234,22 @@ void CvCity::addExport(YieldTypes eYield, bool bUpdateRoutes) kRoutePlayer.addTradeRoute(getIDInfo(), IDInfo((PlayerTypes) iRoutePlayer, CvTradeRoute::EUROPE_CITY_ID), eYield); } } + + if (kRoutePlayer.isYieldAfricaTradable(eYield) && kRoutePlayer.canTradeWithAfrica()) + { + if (isHuman() || isBestPortCity()) + { + kRoutePlayer.addTradeRoute(getIDInfo(), IDInfo((PlayerTypes) iRoutePlayer, CvTradeRoute::AFRICA_CITY_ID), eYield); + } + } + + if (kRoutePlayer.isYieldPortRoyalTradable(eYield) && kRoutePlayer.canTradeWithPortRoyal()) + { + if (isHuman() || isBestPortCity()) + { + kRoutePlayer.addTradeRoute(getIDInfo(), IDInfo((PlayerTypes) iRoutePlayer, CvTradeRoute::PORT_ROYAL_CITY_ID), eYield); + } + } } } } diff --git a/Project Files/DLLSources/CvDLLButtonPopup.cpp b/Project Files/DLLSources/CvDLLButtonPopup.cpp index d4ffe1858..d2ba82ad7 100644 --- a/Project Files/DLLSources/CvDLLButtonPopup.cpp +++ b/Project Files/DLLSources/CvDLLButtonPopup.cpp @@ -1087,11 +1087,14 @@ void CvDLLButtonPopup::OnOkClicked(CvPopup* pPopup, PopupReturn *pPopupReturn, C std::vector routeDetails = split(tradeRouteStr, ' '); int srcId = atoi(routeDetails[0].c_str()); int destId = atoi(routeDetails[1].c_str()); - IDInfo europeCity((PlayerTypes)GC.getGameINLINE().getActivePlayer(),CvTradeRoute::EUROPE_CITY_ID); + PlayerTypes eActivePlayer = (PlayerTypes)GC.getGameINLINE().getActivePlayer(); - addedTradeGroup->addRoute( - srcId != CvTradeRoute::EUROPE_CITY_ID ? player.getCity(srcId)->getIDInfo() : europeCity, - destId != CvTradeRoute::EUROPE_CITY_ID ? player.getCity(destId)->getIDInfo() : europeCity, + IDInfo srcInfo = CvTradeRoute::isOffMapTradeLocation(srcId) ? + IDInfo(eActivePlayer, srcId) : player.getCity(srcId)->getIDInfo(); + IDInfo destInfo = CvTradeRoute::isOffMapTradeLocation(destId) ? + IDInfo(eActivePlayer, destId) : player.getCity(destId)->getIDInfo(); + + addedTradeGroup->addRoute(srcInfo, destInfo, (YieldTypes) atoi(routeDetails[2].c_str())); } diff --git a/Project Files/DLLSources/CvPlayer.cpp b/Project Files/DLLSources/CvPlayer.cpp index 5f45433f1..577b4666e 100644 --- a/Project Files/DLLSources/CvPlayer.cpp +++ b/Project Files/DLLSources/CvPlayer.cpp @@ -10922,7 +10922,7 @@ int CvPlayer::addTradeRoute(const IDInfo& kSource, const IDInfo& kDestination, Y FAssert(pSourceCity->getTeam() == getTeam()); CvCity* pDestinationCity = ::getCity(kDestination); - FAssert(pDestinationCity != NULL || (kDestination.eOwner == getID() && kDestination.iID == CvTradeRoute::EUROPE_CITY_ID)); + FAssert(pDestinationCity != NULL || (kDestination.eOwner == getID() && CvTradeRoute::isOffMapTradeLocation(kDestination.iID))); FAssert(pDestinationCity == NULL || pDestinationCity->getTeam() == getTeam()); if (kSource == kDestination) @@ -11049,7 +11049,7 @@ void CvPlayer::validateTradeRoutes() } } - //re-add missing europe destination routes + //re-add missing off-map destination routes (Europe, Africa, Port Royal) for (int iPlayer = 0; iPlayer < MAX_PLAYERS; ++iPlayer) { CvPlayer& kLoopPlayer = GET_PLAYER((PlayerTypes) iPlayer); @@ -11058,18 +11058,28 @@ void CvPlayer::validateTradeRoutes() for (int iYield = 0; iYield < NUM_YIELD_TYPES; iYield++) { YieldTypes eYield = (YieldTypes) iYield; - if (isYieldEuropeTradable(eYield)) + int iLoop; + for (CvCity* pLoopCity = kLoopPlayer.firstCity(&iLoop); NULL != pLoopCity; pLoopCity = kLoopPlayer.nextCity(&iLoop)) { - int iLoop; - for (CvCity* pLoopCity = kLoopPlayer.firstCity(&iLoop); NULL != pLoopCity; pLoopCity = kLoopPlayer.nextCity(&iLoop)) + // TAC - AI Economy - koma13 - START + //if (pLoopCity->isExport(eYield)) + if (!(pLoopCity->isExport(eYield) && pLoopCity->isBestPortCity())) + // TAC - AI Economy - koma13 - END { - // TAC - AI Economy - koma13 - START - //if (pLoopCity->isExport(eYield)) - if (pLoopCity->isExport(eYield) && pLoopCity->isBestPortCity()) - // TAC - AI Economy - koma13 - END - { - addTradeRoute(pLoopCity->getIDInfo(), IDInfo(getID(), CvTradeRoute::EUROPE_CITY_ID), eYield); - } + continue; + } + + if (isYieldEuropeTradable(eYield)) + { + addTradeRoute(pLoopCity->getIDInfo(), IDInfo(getID(), CvTradeRoute::EUROPE_CITY_ID), eYield); + } + if (isYieldAfricaTradable(eYield) && canTradeWithAfrica()) + { + addTradeRoute(pLoopCity->getIDInfo(), IDInfo(getID(), CvTradeRoute::AFRICA_CITY_ID), eYield); + } + if (isYieldPortRoyalTradable(eYield) && canTradeWithPortRoyal()) + { + addTradeRoute(pLoopCity->getIDInfo(), IDInfo(getID(), CvTradeRoute::PORT_ROYAL_CITY_ID), eYield); } } } @@ -11135,9 +11145,8 @@ namespace void canonicalizePair(const IDInfo& s, const IDInfo& d, IDInfo& outFirst, IDInfo& outSecond) { - // Preserve direction whenever Europe is involved. - // Europe uses the special city id: CvTradeRoute::EUROPE_CITY_ID. - if (s.iID == CvTradeRoute::EUROPE_CITY_ID || d.iID == CvTradeRoute::EUROPE_CITY_ID) + // Preserve direction whenever an off-map trade location is involved. + if (CvTradeRoute::isOffMapTradeLocation(s.iID) || CvTradeRoute::isOffMapTradeLocation(d.iID)) { outFirst = s; // keep as (source, destination) outSecond = d; @@ -11205,13 +11214,13 @@ std::vector CvPlayer::getViableTradeRoutesForUnit(const CvUnit& k if (b == buckets.end() || b->second.empty()) continue; - const bool bEurope = (ok->second.iID == CvTradeRoute::EUROPE_CITY_ID); + const bool bOffMap = CvTradeRoute::isOffMapTradeLocation(ok->second.iID); // Build the list we will emit from this bucket (and the representative ID for the single check) int repId = -1; std::vector emit; - if (!bEurope) + if (!bOffMap) { // City↔city: use the first route as representative (yield-independent checks) repId = b->second.front()->getID(); @@ -11219,18 +11228,29 @@ std::vector CvPlayer::getViableTradeRoutesForUnit(const CvUnit& k } else { - // Europe: only yields tradable to Europe are candidates. + // Off-map destination: only yields tradable at that location are candidates. emit.reserve(b->second.size()); for (size_t i = 0; i < b->second.size(); ++i) { CvTradeRoute* const r = b->second[i]; - if (r && kPlayer.isYieldEuropeTradable(r->getYield())) + if (r == NULL) + continue; + + bool bTradable = false; + if (ok->second.iID == CvTradeRoute::EUROPE_CITY_ID) + bTradable = kPlayer.isYieldEuropeTradable(r->getYield()); + else if (ok->second.iID == CvTradeRoute::AFRICA_CITY_ID) + bTradable = kPlayer.isYieldAfricaTradable(r->getYield()); + else if (ok->second.iID == CvTradeRoute::PORT_ROYAL_CITY_ID) + bTradable = kPlayer.isYieldPortRoyalTradable(r->getYield()); + + if (bTradable) emit.push_back(r); } if (emit.empty()) - continue; // nothing tradable in this (src→Europe) bucket + continue; // nothing tradable in this off-map bucket - // Representative route must use a tradable yield (so canAssign passes Europe checks) + // Representative route must use a tradable yield (so canAssign passes checks) repId = emit.front()->getID(); } diff --git a/Project Files/DLLSources/CvPlayerAI.cpp b/Project Files/DLLSources/CvPlayerAI.cpp index 7864907a0..be717fe86 100644 --- a/Project Files/DLLSources/CvPlayerAI.cpp +++ b/Project Files/DLLSources/CvPlayerAI.cpp @@ -9889,7 +9889,6 @@ int CvPlayerAI::AI_transferYieldValue(const IDInfo target, YieldTypes eYield, in FAssertMsg(eYield > NO_YIELD, "Index out of bounds"); FAssertMsg(eYield < NUM_YIELD_TYPES, "Index out of bounds"); - const IDInfo kEurope(getID(), CvTradeRoute::EUROPE_CITY_ID); CvCity* pCity = ::getCity(target); int iValue = 0; @@ -10021,7 +10020,7 @@ int CvPlayerAI::AI_transferYieldValue(const IDInfo target, YieldTypes eYield, in // transport feeder - end - Nightinggale } } - else if (target == kEurope) + else if (CvTradeRoute::isOffMapTradeLocation(target.iID)) { if (iAmount < 0) //Loading { diff --git a/Project Files/DLLSources/CvSelectionGroupAI.cpp b/Project Files/DLLSources/CvSelectionGroupAI.cpp index daf4fbc0a..7e68e9668 100644 --- a/Project Files/DLLSources/CvSelectionGroupAI.cpp +++ b/Project Files/DLLSources/CvSelectionGroupAI.cpp @@ -910,8 +910,6 @@ bool CvSelectionGroupAI::AI_tradeRoutes() { PROFILE_FUNC(); - const IDInfo kEurope(getOwnerINLINE(), CvTradeRoute::EUROPE_CITY_ID); - CvCity* pPlotCity = plot()->getPlotCity(); CvPlayerAI& kOwner = GET_PLAYER(getOwnerINLINE()); std::set::iterator it; @@ -967,16 +965,16 @@ bool CvSelectionGroupAI::AI_tradeRoutes() // transport feeder - end - Nightinggale // traderoute fix - start - Nightinggale - if (isHuman() && (pRoute->getDestinationCity().eOwner != getOwnerINLINE() || (pRoute->getDestinationCity() == kEurope))) + if (isHuman() && (pRoute->getDestinationCity().eOwner != getOwnerINLINE() || CvTradeRoute::isOffMapTradeLocation(pRoute->getDestinationCity().iID))) { // humans can't transport to allied cities with fully automated transports - // human transport can't go to Europe automatically + // human transport can't go to off-map destinations automatically continue; } // traderoute fix - end - Nightinggale - // Erik: Coastal transports cannot have europe as their destination - if (bCoastalTransport && (pRoute->getDestinationCity().eOwner != getOwnerINLINE() || (pRoute->getDestinationCity() == kEurope))) + // Erik: Coastal transports cannot have off-map destinations + if (bCoastalTransport && (pRoute->getDestinationCity().eOwner != getOwnerINLINE() || CvTradeRoute::isOffMapTradeLocation(pRoute->getDestinationCity().iID))) continue; CvCity* pSourceCity = ::getCity(pRoute->getSourceCity()); @@ -991,7 +989,7 @@ bool CvSelectionGroupAI::AI_tradeRoutes() if (domainType == DOMAIN_SEA ? plot()->isAdjacentToArea(iSourceArea) : (iSourceArea == getArea()) || domainType == DOMAIN_LAND && plot()->getTerrainType() == TERRAIN_LARGE_RIVERS && plot()->isAdjacentToArea(pSourceCity->getArea())) { - if ((domainType == DOMAIN_SEA) || (pRoute->getDestinationCity() != kEurope)) + if ((domainType == DOMAIN_SEA) || !CvTradeRoute::isOffMapTradeLocation(pRoute->getDestinationCity().iID)) { processTradeRoute(pRoute, cityValues, routes, routeValues, yieldsDelivered, yieldsToUnload); } @@ -1017,7 +1015,7 @@ bool CvSelectionGroupAI::AI_tradeRoutes() if (domainType == DOMAIN_SEA ? plot()->isAdjacentToArea(iSourceArea) : (iSourceArea == getArea()) || domainType == DOMAIN_LAND && plot()->getTerrainType() == TERRAIN_LARGE_RIVERS && plot()->isAdjacentToArea(pSourceCity->getArea())) { - if ((domainType == DOMAIN_SEA) || (pRoute->getDestinationCity() != kEurope)) + if ((domainType == DOMAIN_SEA) || !CvTradeRoute::isOffMapTradeLocation(pRoute->getDestinationCity().iID)) { processTradeRoute(pRoute, cityValues, routes, routeValues, yieldsDelivered, yieldsToUnload); } @@ -1174,11 +1172,11 @@ bool CvSelectionGroupAI::AI_tradeRoutes() iAmount = estimateYieldsToLoad(pDestinationCity, iAmount, eYield, turnsRequired, aiYieldsLoaded[eYield]); } - // Note that Europe has no import limit! - else if (pDestinationCity == NULL && routes[i]->getDestinationCity() == kEurope) + // Off-map trade locations have no import limit! + else if (pDestinationCity == NULL && CvTradeRoute::isOffMapTradeLocation(routes[i]->getDestinationCity().iID)) { - // This is a Europe trade-route, exempt it from the reachability criteria - // TODO: Check that there is actually a route to Europe! + // This is an off-map trade route, exempt it from the reachability criteria + // TODO: Check that there is actually a route to the destination! bNoRoute = false; } @@ -1406,7 +1404,7 @@ bool CvSelectionGroupAI::AI_tradeRoutes() } //As a final step, we could consider loading yields which would be useful as parts of delivery runs... - if (kBestDestination != kEurope) + if (!CvTradeRoute::isOffMapTradeLocation(kBestDestination.iID)) { CvCity* pBestDestinationCity = ::getCity(kBestDestination); if (pBestDestinationCity != NULL) diff --git a/Project Files/DLLSources/CvTradeRoute.cpp b/Project Files/DLLSources/CvTradeRoute.cpp index 7ac8bd45d..78ed74a57 100644 --- a/Project Files/DLLSources/CvTradeRoute.cpp +++ b/Project Files/DLLSources/CvTradeRoute.cpp @@ -72,6 +72,14 @@ const wchar* CvTradeRoute::getSourceCityNameKey() const { return L"TXT_KEY_CONCEPT_EUROPE"; } + if (getSourceCity().iID == AFRICA_CITY_ID) + { + return L"TXT_KEY_CONCEPT_AFRICA"; + } + if (getSourceCity().iID == PORT_ROYAL_CITY_ID) + { + return L"TXT_KEY_CONCEPT_PORT_ROYAL"; + } CvCity* pCity = ::getCity(getSourceCity()); FAssert(pCity != NULL); @@ -98,14 +106,14 @@ void CvTradeRoute::setDestinationCity(const IDInfo& kCity) m_kDestinationCity = kCity; CvCity* pCity = ::getCity(getDestinationCity()); - FAssert(pCity != NULL || getDestinationCity().iID == EUROPE_CITY_ID); + FAssert(pCity != NULL || isOffMapTradeLocation(getDestinationCity().iID)); if (pCity != NULL) { pCity->updateImport(getYield()); } pCity = ::getCity(kOldCity); - FAssert(pCity != NULL || getDestinationCity().iID == EUROPE_CITY_ID); + FAssert(pCity != NULL || isOffMapTradeLocation(kOldCity.iID)); if (pCity != NULL) { pCity->updateImport(getYield()); @@ -121,6 +129,14 @@ const wchar* CvTradeRoute::getDestinationCityNameKey() const { return L"TXT_KEY_CONCEPT_EUROPE"; } + if (getDestinationCity().iID == AFRICA_CITY_ID) + { + return L"TXT_KEY_CONCEPT_AFRICA"; + } + if (getDestinationCity().iID == PORT_ROYAL_CITY_ID) + { + return L"TXT_KEY_CONCEPT_PORT_ROYAL"; + } CvCity* pCity = ::getCity(getDestinationCity()); FAssert(pCity != NULL); @@ -187,6 +203,20 @@ bool CvTradeRoute::checkValid(PlayerTypes ePlayer) const return false; } } + else if (getDestinationCity().iID == AFRICA_CITY_ID) + { + if (!kPlayer.isYieldAfricaTradable(getYield()) || !kPlayer.canTradeWithAfrica()) + { + return false; + } + } + else if (getDestinationCity().iID == PORT_ROYAL_CITY_ID) + { + if (!kPlayer.isYieldPortRoyalTradable(getYield()) || !kPlayer.canTradeWithPortRoyal()) + { + return false; + } + } return true; } diff --git a/Project Files/DLLSources/CvTradeRoute.h b/Project Files/DLLSources/CvTradeRoute.h index 6020de434..c9cf1e961 100644 --- a/Project Files/DLLSources/CvTradeRoute.h +++ b/Project Files/DLLSources/CvTradeRoute.h @@ -10,6 +10,13 @@ class CvTradeRoute public: static const int EUROPE_CITY_ID = -1; static const int ANYWHERE_CITY_ID = -2; + static const int AFRICA_CITY_ID = -3; + static const int PORT_ROYAL_CITY_ID = -4; + + static bool isOffMapTradeLocation(int iID) + { + return iID == EUROPE_CITY_ID || iID == AFRICA_CITY_ID || iID == PORT_ROYAL_CITY_ID; + } CvTradeRoute(); ~CvTradeRoute(); diff --git a/Project Files/DLLSources/CvTradeRouteGroup.h b/Project Files/DLLSources/CvTradeRouteGroup.h index 7ba3dd263..94bcd0647 100644 --- a/Project Files/DLLSources/CvTradeRouteGroup.h +++ b/Project Files/DLLSources/CvTradeRouteGroup.h @@ -26,9 +26,6 @@ class CvTradeRouteGroup void write(CvSavegameWriter writer); void resetSavedData(); - - static const int EUROPE_CITY_ID = -1; - static const int ANYWHERE_CITY_ID = -2; protected: int m_iId; diff --git a/Project Files/DLLSources/CvUnit.cpp b/Project Files/DLLSources/CvUnit.cpp index 03871a4d7..50eaa774d 100644 --- a/Project Files/DLLSources/CvUnit.cpp +++ b/Project Files/DLLSources/CvUnit.cpp @@ -10418,17 +10418,36 @@ bool CvUnit::canAssignTradeRoute(int iRouteID, bool bReusePath) const if (!pf.GeneratePath(pSource->plot())) return false; - // Europe destination special case (no map city) - if (kDst.iID == CvTradeRoute::EUROPE_CITY_ID) + // Off-map destination special case (Europe, Africa, Port Royal) + if (CvTradeRoute::isOffMapTradeLocation(kDst.iID)) { if (canCrossCoastOnly()) return false; if (getDomainType() != DOMAIN_SEA) return false; - if (!kPlayer.isYieldEuropeTradable(pRoute->getYield())) - return false; - // TODO: Perform actual pathfinding to a reachable Europe plot + if (kDst.iID == CvTradeRoute::EUROPE_CITY_ID) + { + if (!kPlayer.isYieldEuropeTradable(pRoute->getYield())) + return false; + } + else if (kDst.iID == CvTradeRoute::AFRICA_CITY_ID) + { + if (!kPlayer.isYieldAfricaTradable(pRoute->getYield())) + return false; + if (!kPlayer.canTradeWithAfrica()) + return false; + } + else if (kDst.iID == CvTradeRoute::PORT_ROYAL_CITY_ID) + { + if (!kPlayer.isYieldPortRoyalTradable(pRoute->getYield())) + return false; + if (!kPlayer.canTradeWithPortRoyal()) + return false; + if (!canSailToPortRoyal(plot())) + return false; + } + return true; } diff --git a/Project Files/DLLSources/CyUnit.cpp b/Project Files/DLLSources/CyUnit.cpp index f28215231..be9d660cd 100644 --- a/Project Files/DLLSources/CyUnit.cpp +++ b/Project Files/DLLSources/CyUnit.cpp @@ -1091,6 +1091,14 @@ bool CyUnit::canSailEurope(int iEurope) { return m_pUnit ? m_pUnit->canSailEurope((EuropeTypes) iEurope) : false; } +bool CyUnit::canSailToAfrica() +{ + return m_pUnit ? m_pUnit->canSailToAfrica(m_pUnit->plot()) : false; +} +bool CyUnit::canSailToPortRoyal() +{ + return m_pUnit ? m_pUnit->canSailToPortRoyal(m_pUnit->plot()) : false; +} bool CyUnit::isColonistLocked() { return m_pUnit ? m_pUnit->isColonistLocked() : false; diff --git a/Project Files/DLLSources/CyUnit.h b/Project Files/DLLSources/CyUnit.h index 6fd4b03e1..19dd3aed1 100644 --- a/Project Files/DLLSources/CyUnit.h +++ b/Project Files/DLLSources/CyUnit.h @@ -276,6 +276,8 @@ class CyUnit int /*UnitTravelStates*/ getUnitTravelState(); void setUnitTravelState(int /*UnitTravelStates*/ eState, bool bShowEuropeScreen); bool canSailEurope(int iEurope); + bool canSailToAfrica(); + bool canSailToPortRoyal(); bool isColonistLocked(); // < JAnimals Mod Start > bool isBarbarian(); diff --git a/Project Files/DLLSources/CyUnitInterface2.cpp b/Project Files/DLLSources/CyUnitInterface2.cpp index 5a9687c49..91d738518 100644 --- a/Project Files/DLLSources/CyUnitInterface2.cpp +++ b/Project Files/DLLSources/CyUnitInterface2.cpp @@ -178,6 +178,8 @@ void CyUnitPythonInterface2(python::class_& x) .def("getUnitTravelState", &CyUnit::getUnitTravelState, "int /*UnitTravelStates*/ ()") .def("setUnitTravelState", &CyUnit::setUnitTravelState, "void (int /*UnitTravelStates*/, bool bShowEuropeScreen)") .def("canSailEurope", &CyUnit::canSailEurope) + .def("canSailToAfrica", &CyUnit::canSailToAfrica, "bool ()") + .def("canSailToPortRoyal", &CyUnit::canSailToPortRoyal, "bool ()") .def("isColonistLocked", &CyUnit::isColonistLocked, "bool ()") // < JAnimals Mod Start > .def("isBarbarian", &CyUnit::isBarbarian, "bool ()") From fa7e992c7c8094bf297c7f8ef3413ed251072bb5 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 19:46:41 -0700 Subject: [PATCH 02/13] Add WTP test framework and data structure tests (Phase 1) Custom MinUnit-derived C++03 test framework with 35 tests across 6 suites covering EnumMap, JustInTimeArray, Coordinates, CvTradeRoute sentinels, and CvIdVector. Tests run at startup in Assert/Debug builds after XML load, logging results to Logs\WTPTests.log. Co-Authored-By: Claude Opus 4.6 --- .../DLLSources/CvGlobalsEnumSetup.cpp | 8 + .../DLLSources/TestDataStructures.cpp | 586 ++++++++++++++++++ Project Files/DLLSources/WTPTestFramework.cpp | 47 ++ Project Files/DLLSources/WTPTestFramework.h | 93 +++ Project Files/RaR.vcxproj | 2 + 5 files changed, 736 insertions(+) create mode 100644 Project Files/DLLSources/TestDataStructures.cpp create mode 100644 Project Files/DLLSources/WTPTestFramework.cpp create mode 100644 Project Files/DLLSources/WTPTestFramework.h diff --git a/Project Files/DLLSources/CvGlobalsEnumSetup.cpp b/Project Files/DLLSources/CvGlobalsEnumSetup.cpp index d0b18b725..35c9d69ec 100644 --- a/Project Files/DLLSources/CvGlobalsEnumSetup.cpp +++ b/Project Files/DLLSources/CvGlobalsEnumSetup.cpp @@ -8,6 +8,7 @@ #include "autogenerated/AutoXmlDeclare.h" void TestEnumMap(); +void RunAllWTPTests(); static void DisplayXMLmissingError(bool bSuccess, const char* szName) @@ -112,6 +113,13 @@ void CvGlobals::postXMLLoad(bool bFirst) TestEnumMap(); } #endif + +#ifdef FASSERT_ENABLE + if (!bFirst) + { + RunAllWTPTests(); + } +#endif } diff --git a/Project Files/DLLSources/TestDataStructures.cpp b/Project Files/DLLSources/TestDataStructures.cpp new file mode 100644 index 000000000..5501ffc55 --- /dev/null +++ b/Project Files/DLLSources/TestDataStructures.cpp @@ -0,0 +1,586 @@ + +#include "CvGameCoreDLL.h" +#include "WTPTestFramework.h" + +#ifdef FASSERT_ENABLE + +#include "CvTradeRoute.h" +#include "CvIdVector.h" + +// ============================================================================ +// EnumMap Tests +// ============================================================================ + +// --- int specialization --- + +WTP_TEST(EnumMap, IntDefaultZero) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + WTP_ASSERT(!em.isAllocated()); + WTP_ASSERT(!em.hasContent()); + WTP_ASSERT_EQ(0, em.get(eKey)); + WTP_ASSERT_EQ(0, em.getTotal()); +} + +WTP_TEST(EnumMap, IntSetGetReset) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + em.set(eKey, 42); + WTP_ASSERT(em.isAllocated()); + WTP_ASSERT(em.hasContent()); + WTP_ASSERT_EQ(42, em.get(eKey)); + + em.set(eKey, 0); + WTP_ASSERT_EQ(0, em.get(eKey)); + WTP_ASSERT(!em.hasContent()); + + em.set(eKey, 7); + em.reset(); + WTP_ASSERT(!em.isAllocated()); + WTP_ASSERT(!em.hasContent()); + WTP_ASSERT_EQ(0, em.get(eKey)); +} + +WTP_TEST(EnumMap, IntAdd) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + em.add(eKey, 5); + WTP_ASSERT_EQ(5, em.get(eKey)); + em.add(eKey, 3); + WTP_ASSERT_EQ(8, em.get(eKey)); + em.add(eKey, -8); + WTP_ASSERT_EQ(0, em.get(eKey)); +} + +WTP_TEST(EnumMap, IntGetTotal) +{ + EnumMap em; + RouteTypes e0 = static_cast(0); + RouteTypes e1 = static_cast(1); + + em.set(e0, 10); + em.set(e1, 20); + WTP_ASSERT_EQ(30, em.getTotal()); +} + +WTP_TEST(EnumMap, IntAddAll) +{ + EnumMap em; + RouteTypes e0 = static_cast(0); + + em.addAll(5); + WTP_ASSERT_EQ(5, em.get(e0)); + WTP_ASSERT_EQ(NUM_ROUTE_TYPES * 5, em.getTotal()); +} + +WTP_TEST(EnumMap, IntNonZeroDefault) +{ + EnumMap em; + RouteTypes e0 = static_cast(0); + + WTP_ASSERT_EQ(10, em.get(e0)); + WTP_ASSERT_EQ(NUM_ROUTE_TYPES * 10, em.getTotal()); + + em.set(e0, 0); + WTP_ASSERT_EQ(0, em.get(e0)); + WTP_ASSERT_EQ((NUM_ROUTE_TYPES - 1) * 10, em.getTotal()); +} + +// --- bool specialization (bitfield) --- + +WTP_TEST(EnumMap, BoolDefault) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + WTP_ASSERT(!em.get(eKey)); + WTP_ASSERT(!em.hasContent()); + WTP_ASSERT_EQ(0, em.getTotal()); +} + +WTP_TEST(EnumMap, BoolSetGetReset) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + em.set(eKey, true); + WTP_ASSERT(em.get(eKey)); + WTP_ASSERT(em.hasContent()); + WTP_ASSERT_EQ(1, em.getTotal()); + + em.set(eKey, false); + WTP_ASSERT(!em.get(eKey)); + WTP_ASSERT(!em.hasContent()); + + em.set(eKey, true); + em.reset(); + WTP_ASSERT(!em.get(eKey)); + WTP_ASSERT(!em.hasContent()); +} + +WTP_TEST(EnumMap, BoolMultipleKeys) +{ + EnumMap em; + RouteTypes e0 = static_cast(0); + RouteTypes e1 = static_cast(1); + + em.set(e0, true); + em.set(e1, true); + WTP_ASSERT_EQ(2, em.getTotal()); + WTP_ASSERT(em.get(e0)); + WTP_ASSERT(em.get(e1)); + + em.set(e0, false); + WTP_ASSERT_EQ(1, em.getTotal()); + WTP_ASSERT(!em.get(e0)); + WTP_ASSERT(em.get(e1)); +} + +// --- enum value type --- + +WTP_TEST(EnumMap, EnumValueType) +{ + EnumMap em; + RouteTypes eKey = static_cast(0); + + WTP_ASSERT_EQ(NO_PLAYER, em.get(eKey)); + WTP_ASSERT(!em.hasContent()); + + em.set(eKey, FIRST_PLAYER); + WTP_ASSERT_EQ(FIRST_PLAYER, em.get(eKey)); + WTP_ASSERT(em.hasContent()); + + em.set(eKey, NO_PLAYER); + WTP_ASSERT(!em.hasContent()); + + em.set(eKey, FIRST_PLAYER); + em.reset(); + WTP_ASSERT_EQ(NO_PLAYER, em.get(eKey)); + WTP_ASSERT(!em.hasContent()); +} + +// --- Large key type (UnitTypes has many entries) --- + +WTP_TEST(EnumMap, LargeKeyType) +{ + EnumMap em; + UnitTypes eKey = static_cast(1); + + WTP_ASSERT_EQ(NUM_UNIT_TYPES * 10, em.getTotal()); + em.set(eKey, 0); + WTP_ASSERT_EQ((NUM_UNIT_TYPES * 10) - 10, em.getTotal()); + em.addAll(1); + WTP_ASSERT_EQ((NUM_UNIT_TYPES * 11) - 10, em.getTotal()); +} + +// --- Full iteration --- + +WTP_TEST(EnumMap, FullIteration) +{ + EnumMap em; + + // Set all values + for (RouteTypes e = FIRST_ROUTE; e < NUM_ROUTE_TYPES; ++e) + { + em.set(e, static_cast(e) + 1); + } + + // Verify all values + for (RouteTypes e = FIRST_ROUTE; e < NUM_ROUTE_TYPES; ++e) + { + WTP_ASSERT_EQ(static_cast(e) + 1, em.get(e)); + } + + WTP_ASSERT(em.hasContent()); +} + +// ============================================================================ +// JustInTimeArray Tests +// ============================================================================ + +WTP_TEST(JustInTimeArray, DefaultUnallocated) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + WTP_ASSERT(!arr.isAllocated()); + WTP_ASSERT_EQ(0, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, SetAllocates) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(5, 0); + WTP_ASSERT(arr.isAllocated()); + WTP_ASSERT_EQ(5, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, SetDefaultNoAllocate) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(0, 0); + // Setting the default value should not allocate (or should deallocate) + // The array may or may not remain allocated, but the value should be default + WTP_ASSERT_EQ(0, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, ResetDeallocates) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(42, 0); + WTP_ASSERT(arr.isAllocated()); + arr.reset(); + WTP_ASSERT(!arr.isAllocated()); + WTP_ASSERT_EQ(0, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, AddAccumulates) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(10, 0); + arr.add(5, 0); + WTP_ASSERT_EQ(15, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, HasContent) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + WTP_ASSERT(!arr.hasContent()); + arr.set(1, 0); + WTP_ASSERT(arr.hasContent()); + arr.set(0, 0); + // After setting back to default, hasContent may still return true if allocated + // but reset should clear it + arr.reset(); + WTP_ASSERT(!arr.hasContent()); +} + +WTP_TEST(JustInTimeArray, NonZeroDefault) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE, 7); + + WTP_ASSERT(!arr.isAllocated()); + WTP_ASSERT_EQ(7, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, MultipleIndices) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + int iLen = arr.length(); + + WTP_ASSERT(iLen > 0); + + arr.set(100, 0); + if (iLen > 1) + { + arr.set(200, 1); + WTP_ASSERT_EQ(100, arr.get(0)); + WTP_ASSERT_EQ(200, arr.get(1)); + } +} + +WTP_TEST(JustInTimeArray, KeepMax) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(10, 0); + arr.keepMax(5, 0); + WTP_ASSERT_EQ(10, arr.get(0)); + arr.keepMax(20, 0); + WTP_ASSERT_EQ(20, arr.get(0)); +} + +WTP_TEST(JustInTimeArray, KeepMin) +{ + JustInTimeArray arr(JIT_ARRAY_ROUTE); + + arr.set(10, 0); + arr.keepMin(20, 0); + WTP_ASSERT_EQ(10, arr.get(0)); + arr.keepMin(5, 0); + WTP_ASSERT_EQ(5, arr.get(0)); +} + +// ============================================================================ +// Coordinates Tests +// ============================================================================ + +WTP_TEST(Coordinates, DefaultIsInvalid) +{ + Coordinates c; + WTP_ASSERT(c.isInvalidPlotCoord()); +} + +WTP_TEST(Coordinates, ExplicitConstruction) +{ + Coordinates c(3, 5); + // Note: Coordinates::set() calls resetInvalid() if not on map, + // so we can only test with valid coords when a map exists. + // Without a map, they become invalid. Test the invalid path. + Coordinates inv; + WTP_ASSERT(inv.isInvalidPlotCoord()); +} + +WTP_TEST(Coordinates, InvalidCoordFactory) +{ + Coordinates c = Coordinates::invalidCoord(); + WTP_ASSERT(c.isInvalidPlotCoord()); +} + +WTP_TEST(Coordinates, NullCoordFactory) +{ + Coordinates c = Coordinates::nullCoord(); + // nullCoord should not be invalid plot coord (it's 0,0 conceptually) + // but without a map it may get reset to invalid + // Just verify it doesn't crash + (void)c.x(); + (void)c.y(); +} + +WTP_TEST(Coordinates, EqualityInvalidCoords) +{ + Coordinates a; + Coordinates b; + // Both default-constructed (invalid) should be equal + WTP_ASSERT(a == b); + WTP_ASSERT(!(a != b)); +} + +// ============================================================================ +// CvTradeRoute Sentinel Tests +// ============================================================================ + +WTP_TEST(TradeRoute, SentinelConstants) +{ + // Verify sentinel values are distinct + WTP_ASSERT(CvTradeRoute::EUROPE_CITY_ID != CvTradeRoute::AFRICA_CITY_ID); + WTP_ASSERT(CvTradeRoute::EUROPE_CITY_ID != CvTradeRoute::PORT_ROYAL_CITY_ID); + WTP_ASSERT(CvTradeRoute::AFRICA_CITY_ID != CvTradeRoute::PORT_ROYAL_CITY_ID); + WTP_ASSERT(CvTradeRoute::EUROPE_CITY_ID != CvTradeRoute::ANYWHERE_CITY_ID); + WTP_ASSERT(CvTradeRoute::AFRICA_CITY_ID != CvTradeRoute::ANYWHERE_CITY_ID); + WTP_ASSERT(CvTradeRoute::PORT_ROYAL_CITY_ID != CvTradeRoute::ANYWHERE_CITY_ID); + + // All sentinels are negative + WTP_ASSERT(CvTradeRoute::EUROPE_CITY_ID < 0); + WTP_ASSERT(CvTradeRoute::AFRICA_CITY_ID < 0); + WTP_ASSERT(CvTradeRoute::PORT_ROYAL_CITY_ID < 0); + WTP_ASSERT(CvTradeRoute::ANYWHERE_CITY_ID < 0); +} + +WTP_TEST(TradeRoute, IsOffMapTradeLocation) +{ + // These three should be recognized as off-map + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::EUROPE_CITY_ID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::AFRICA_CITY_ID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::PORT_ROYAL_CITY_ID)); + + // ANYWHERE is NOT an off-map trade location (by design) + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::ANYWHERE_CITY_ID)); + + // Regular city IDs (>= 0) are not off-map + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(0)); + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(1)); + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(100)); + + // Arbitrary negative values that aren't sentinels + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(-5)); + WTP_ASSERT(!CvTradeRoute::isOffMapTradeLocation(-100)); +} + +WTP_TEST(TradeRoute, DefaultConstruction) +{ + CvTradeRoute route; + // Default-constructed route should have invalid/default state + WTP_ASSERT_EQ(NO_YIELD, route.getYield()); +} + +// ============================================================================ +// CvIdVector Tests +// ============================================================================ + +// Helper class for CvIdVector testing +class TestIdItem +{ +public: + TestIdItem() : m_iId(-1), m_iValue(0) {} + + int getID() const { return m_iId; } + void setID(int iId) { m_iId = iId; } + + int getValue() const { return m_iValue; } + void setValue(int v) { m_iValue = v; } + + void read(FDataStreamBase*) {} + void write(FDataStreamBase*) const {} + +private: + int m_iId; + int m_iValue; +}; + +WTP_TEST(CvIdVector, EmptyAfterConstruction) +{ + CvIdVector vec; + WTP_ASSERT(vec.empty()); + WTP_ASSERT(vec.getById(0) == NULL); +} + +WTP_TEST(CvIdVector, AddNewAssignsSequentialIds) +{ + CvIdVector vec; + + TestIdItem* p0 = vec.addNew(); + WTP_ASSERT(p0 != NULL); + WTP_ASSERT_EQ(0, p0->getID()); + + TestIdItem* p1 = vec.addNew(); + WTP_ASSERT(p1 != NULL); + WTP_ASSERT_EQ(1, p1->getID()); + + WTP_ASSERT_EQ(2, static_cast(vec.size())); +} + +WTP_TEST(CvIdVector, GetByIdReturnsCorrectItem) +{ + CvIdVector vec; + + TestIdItem* p0 = vec.addNew(); + p0->setValue(42); + + TestIdItem* p1 = vec.addNew(); + p1->setValue(99); + + TestIdItem* found0 = vec.getById(0); + WTP_ASSERT(found0 != NULL); + WTP_ASSERT_EQ(42, found0->getValue()); + + TestIdItem* found1 = vec.getById(1); + WTP_ASSERT(found1 != NULL); + WTP_ASSERT_EQ(99, found1->getValue()); + + WTP_ASSERT(vec.getById(2) == NULL); +} + +WTP_TEST(CvIdVector, RemoveById) +{ + CvIdVector vec; + vec.addNew(); + vec.addNew(); + + WTP_ASSERT(vec.removeById(0)); + WTP_ASSERT(vec.getById(0) == NULL); + WTP_ASSERT(vec.getById(1) != NULL); + WTP_ASSERT_EQ(1, static_cast(vec.size())); + + // Removing non-existent ID returns false + WTP_ASSERT(!vec.removeById(0)); + WTP_ASSERT(!vec.removeById(999)); +} + +WTP_TEST(CvIdVector, ResetClearsAll) +{ + CvIdVector vec; + vec.addNew(); + vec.addNew(); + vec.addNew(); + + vec.reset(); + WTP_ASSERT(vec.empty()); + WTP_ASSERT(vec.getById(0) == NULL); + WTP_ASSERT(vec.getById(1) == NULL); + WTP_ASSERT(vec.getById(2) == NULL); +} + +WTP_TEST(CvIdVector, IdsNotReusedAfterRemove) +{ + CvIdVector vec; + vec.addNew(); // ID 0 + vec.addNew(); // ID 1 + vec.removeById(0); + + TestIdItem* p2 = vec.addNew(); // should be ID 2, not 0 + WTP_ASSERT_EQ(2, p2->getID()); +} + +// ============================================================================ +// Test Runner +// ============================================================================ + +void RunDataStructureTests() +{ + // EnumMap int tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(EnumMap, IntDefaultZero); + WTP_RUN_TEST(EnumMap, IntSetGetReset); + WTP_RUN_TEST(EnumMap, IntAdd); + WTP_RUN_TEST(EnumMap, IntGetTotal); + WTP_RUN_TEST(EnumMap, IntAddAll); + WTP_RUN_TEST(EnumMap, IntNonZeroDefault); + WTP_RUN_SUITE_END("EnumMap_Int"); + + // EnumMap bool tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(EnumMap, BoolDefault); + WTP_RUN_TEST(EnumMap, BoolSetGetReset); + WTP_RUN_TEST(EnumMap, BoolMultipleKeys); + WTP_RUN_SUITE_END("EnumMap_Bool"); + + // EnumMap enum value tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(EnumMap, EnumValueType); + WTP_RUN_TEST(EnumMap, LargeKeyType); + WTP_RUN_TEST(EnumMap, FullIteration); + WTP_RUN_SUITE_END("EnumMap_Enum"); + + // JustInTimeArray tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(JustInTimeArray, DefaultUnallocated); + WTP_RUN_TEST(JustInTimeArray, SetAllocates); + WTP_RUN_TEST(JustInTimeArray, SetDefaultNoAllocate); + WTP_RUN_TEST(JustInTimeArray, ResetDeallocates); + WTP_RUN_TEST(JustInTimeArray, AddAccumulates); + WTP_RUN_TEST(JustInTimeArray, HasContent); + WTP_RUN_TEST(JustInTimeArray, NonZeroDefault); + WTP_RUN_TEST(JustInTimeArray, MultipleIndices); + WTP_RUN_TEST(JustInTimeArray, KeepMax); + WTP_RUN_TEST(JustInTimeArray, KeepMin); + WTP_RUN_SUITE_END("JustInTimeArray"); + + // Coordinates tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Coordinates, DefaultIsInvalid); + WTP_RUN_TEST(Coordinates, ExplicitConstruction); + WTP_RUN_TEST(Coordinates, InvalidCoordFactory); + WTP_RUN_TEST(Coordinates, NullCoordFactory); + WTP_RUN_TEST(Coordinates, EqualityInvalidCoords); + WTP_RUN_SUITE_END("Coordinates"); + + // CvTradeRoute sentinel tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(TradeRoute, SentinelConstants); + WTP_RUN_TEST(TradeRoute, IsOffMapTradeLocation); + WTP_RUN_TEST(TradeRoute, DefaultConstruction); + WTP_RUN_SUITE_END("TradeRoute"); + + // CvIdVector tests + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(CvIdVector, EmptyAfterConstruction); + WTP_RUN_TEST(CvIdVector, AddNewAssignsSequentialIds); + WTP_RUN_TEST(CvIdVector, GetByIdReturnsCorrectItem); + WTP_RUN_TEST(CvIdVector, RemoveById); + WTP_RUN_TEST(CvIdVector, ResetClearsAll); + WTP_RUN_TEST(CvIdVector, IdsNotReusedAfterRemove); + WTP_RUN_SUITE_END("CvIdVector"); +} + +#endif diff --git a/Project Files/DLLSources/WTPTestFramework.cpp b/Project Files/DLLSources/WTPTestFramework.cpp new file mode 100644 index 000000000..946878dcc --- /dev/null +++ b/Project Files/DLLSources/WTPTestFramework.cpp @@ -0,0 +1,47 @@ + +#include "CvGameCoreDLL.h" +#include "WTPTestFramework.h" + +#ifdef FASSERT_ENABLE + +WTPTestResult g_wtpTestResult; + +static const char* LOG_FILE = "Logs\\WTPTests.log"; + +void WTPTestReport(const char* suiteName, const WTPTestResult& result) +{ + char szBuf[512]; + + if (result.tests_failed == 0) + { + snprintf(szBuf, sizeof(szBuf), "[PASS] %s: %d/%d tests passed", + suiteName, result.tests_passed, result.tests_run); + gDLL->logMsg(LOG_FILE, szBuf); + } + else + { + snprintf(szBuf, sizeof(szBuf), "[FAIL] %s: %d/%d tests failed (first failure: %s at %s:%d)", + suiteName, result.tests_failed, result.tests_run, + result.first_failure_msg ? result.first_failure_msg : "unknown", + result.first_failure_file ? result.first_failure_file : "unknown", + result.first_failure_line); + gDLL->logMsg(LOG_FILE, szBuf); + + // Also fire FAssert so developers see the failure in Debug builds + FAssertMsg(false, szBuf); + } +} + +// forward declarations for test suites +void RunDataStructureTests(); + +void RunAllWTPTests() +{ + gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Starting ==="); + + RunDataStructureTests(); + + gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Complete ==="); +} + +#endif diff --git a/Project Files/DLLSources/WTPTestFramework.h b/Project Files/DLLSources/WTPTestFramework.h new file mode 100644 index 000000000..a48006e64 --- /dev/null +++ b/Project Files/DLLSources/WTPTestFramework.h @@ -0,0 +1,93 @@ +#pragma once + +// WTP Test Framework -- MinUnit-derived, C++03 compatible +// +// Usage: +// WTP_TEST(TestSuiteName, TestName) +// { +// WTP_ASSERT(1 + 1 == 2); +// WTP_ASSERT_MSG(value > 0, "value must be positive"); +// WTP_ASSERT_EQ(expected, actual); +// } +// +// In RunAllWTPTests(): +// WTP_RUN_SUITE_BEGIN(); +// WTP_RUN_TEST(SuiteName, TestName); +// WTP_RUN_SUITE_END("SuiteName"); + +#ifdef FASSERT_ENABLE + +struct WTPTestResult +{ + int tests_run; + int tests_passed; + int tests_failed; + const char* first_failure_file; + int first_failure_line; + const char* first_failure_msg; + + WTPTestResult() : tests_run(0), tests_passed(0), tests_failed(0), + first_failure_file(NULL), first_failure_line(0), + first_failure_msg(NULL) {} +}; + +// Global result accumulator (reset per suite) +extern WTPTestResult g_wtpTestResult; + +#define WTP_TEST(suite, name) \ + static void suite##_##name(WTPTestResult& _result) + +#define WTP_ASSERT(expr) \ + do { \ + _result.tests_run++; \ + if (!(expr)) { \ + _result.tests_failed++; \ + if (_result.first_failure_file == NULL) { \ + _result.first_failure_file = __FILE__; \ + _result.first_failure_line = __LINE__; \ + _result.first_failure_msg = #expr; \ + } \ + } else { \ + _result.tests_passed++; \ + } \ + } while(0) + +#define WTP_ASSERT_MSG(expr, msg) \ + do { \ + _result.tests_run++; \ + if (!(expr)) { \ + _result.tests_failed++; \ + if (_result.first_failure_file == NULL) { \ + _result.first_failure_file = __FILE__; \ + _result.first_failure_line = __LINE__; \ + _result.first_failure_msg = msg; \ + } \ + } else { \ + _result.tests_passed++; \ + } \ + } while(0) + +#define WTP_ASSERT_EQ(expected, actual) \ + WTP_ASSERT_MSG((expected) == (actual), #expected " != " #actual) + +#define WTP_RUN_TEST(suite, name) \ + suite##_##name(g_wtpTestResult) + +#define WTP_RUN_SUITE_BEGIN() \ + g_wtpTestResult = WTPTestResult() + +#define WTP_RUN_SUITE_END(suiteName) \ + WTPTestReport(suiteName, g_wtpTestResult) + +// Report function (implemented in WTPTestFramework.cpp) +void WTPTestReport(const char* suiteName, const WTPTestResult& result); + +// Master test runner (called from CvGlobalsEnumSetup.cpp) +void RunAllWTPTests(); + +#else + +// Compiled out in Release/FinalRelease +inline void RunAllWTPTests() {} + +#endif diff --git a/Project Files/RaR.vcxproj b/Project Files/RaR.vcxproj index 805141d25..fba988e60 100644 --- a/Project Files/RaR.vcxproj +++ b/Project Files/RaR.vcxproj @@ -119,6 +119,8 @@ + + From d76cafe23562e76903f2d53a449ce55963f1d836 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 19:54:01 -0700 Subject: [PATCH 03/13] Add XML integrity tests and build-time validation scripts (Phase 2) C++ tests (10) validate XML cross-references at startup: profession yields, building classes, unit professions, father CivEffects/categories, yield costs, equipment amounts, terrain yields, and trade sentinels. Perl scripts: test_determinism.pl detects non-deterministic calls (rand/srand/time/GetTickCount) outside allowed files; test_text_keys.pl verifies TXT_KEY references in data XML have matching text definitions. Both run as part of the test_DllExport build target. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/TestXMLIntegrity.cpp | 219 ++++++++++++++++++ Project Files/DLLSources/WTPTestFramework.cpp | 2 + Project Files/Makefile | 2 + Project Files/RaR.vcxproj | 1 + Project Files/bin/test_determinism.pl | 91 ++++++++ Project Files/bin/test_text_keys.pl | 106 +++++++++ 6 files changed, 421 insertions(+) create mode 100644 Project Files/DLLSources/TestXMLIntegrity.cpp create mode 100644 Project Files/bin/test_determinism.pl create mode 100644 Project Files/bin/test_text_keys.pl diff --git a/Project Files/DLLSources/TestXMLIntegrity.cpp b/Project Files/DLLSources/TestXMLIntegrity.cpp new file mode 100644 index 000000000..f1316923a --- /dev/null +++ b/Project Files/DLLSources/TestXMLIntegrity.cpp @@ -0,0 +1,219 @@ + +#include "CvGameCoreDLL.h" +#include "WTPTestFramework.h" + +#ifdef FASSERT_ENABLE + +#include "CvTradeRoute.h" + +// ============================================================================ +// Profession Yield Validity +// ============================================================================ + +WTP_TEST(XMLIntegrity, ProfessionYieldsValid) +{ + for (ProfessionTypes eProfession = FIRST_PROFESSION; eProfession < NUM_PROFESSION_TYPES; ++eProfession) + { + const CvProfessionInfo& kProfession = GC.getProfessionInfo(eProfession); + + for (int i = 0; i < kProfession.getNumYieldsProduced(); ++i) + { + int eYield = kProfession.getYieldsProduced(i); + WTP_ASSERT_MSG(eYield >= 0 && eYield < NUM_YIELD_TYPES, + "Profession has invalid produced yield"); + } + + for (int i = 0; i < kProfession.getNumYieldsConsumed(); ++i) + { + int eYield = kProfession.getYieldsConsumed(i); + WTP_ASSERT_MSG(eYield >= 0 && eYield < NUM_YIELD_TYPES, + "Profession has invalid consumed yield"); + } + } +} + +// ============================================================================ +// Building Class Validity +// ============================================================================ + +WTP_TEST(XMLIntegrity, BuildingClassValid) +{ + for (BuildingTypes eBuilding = FIRST_BUILDING; eBuilding < NUM_BUILDING_TYPES; ++eBuilding) + { + const CvBuildingInfo& kBuilding = GC.getBuildingInfo(eBuilding); + int iBuildingClass = kBuilding.getBuildingClassType(); + WTP_ASSERT_MSG(iBuildingClass >= 0 && iBuildingClass < GC.getNumBuildingClassInfos(), + "Building has invalid BuildingClassType"); + } +} + +// ============================================================================ +// Unit Default Professions +// ============================================================================ + +WTP_TEST(XMLIntegrity, UnitDefaultProfessionValid) +{ + for (UnitTypes eUnit = FIRST_UNIT; eUnit < NUM_UNIT_TYPES; ++eUnit) + { + const CvUnitInfo& kUnit = GC.getUnitInfo(eUnit); + ProfessionTypes eDefaultProfession = kUnit.getDefaultProfession(); + + // NO_PROFESSION is valid (some units have no default profession) + if (eDefaultProfession != NO_PROFESSION) + { + WTP_ASSERT_MSG(eDefaultProfession >= FIRST_PROFESSION && eDefaultProfession < NUM_PROFESSION_TYPES, + "Unit has invalid default profession"); + } + } +} + +// ============================================================================ +// Founding Father CivEffect References +// ============================================================================ + +WTP_TEST(XMLIntegrity, FatherCivEffectValid) +{ + for (FatherTypes eFather = FIRST_FATHER; eFather < NUM_FATHER_TYPES; ++eFather) + { + const CvFatherInfo& kFather = GC.getFatherInfo(eFather); + CivEffectTypes eCivEffect = kFather.getCivEffect(); + + // CivEffect should be valid if assigned + if (eCivEffect != NO_CIV_EFFECT) + { + WTP_ASSERT_MSG(eCivEffect >= FIRST_CIV_EFFECT && eCivEffect < NUM_CIV_EFFECT_TYPES, + "Father has invalid CivEffect index"); + } + } +} + +// ============================================================================ +// Father Category References +// ============================================================================ + +WTP_TEST(XMLIntegrity, FatherCategoryValid) +{ + for (FatherTypes eFather = FIRST_FATHER; eFather < NUM_FATHER_TYPES; ++eFather) + { + const CvFatherInfo& kFather = GC.getFatherInfo(eFather); + FatherCategoryTypes eCategory = kFather.getFatherCategory(); + + WTP_ASSERT_MSG(eCategory >= 0 && eCategory < GC.getNumFatherCategoryInfos(), + "Father has invalid category"); + } +} + +// ============================================================================ +// Unit Building Class References (training buildings) +// ============================================================================ + +WTP_TEST(XMLIntegrity, UnitYieldCostsNonNegative) +{ + for (UnitTypes eUnit = FIRST_UNIT; eUnit < NUM_UNIT_TYPES; ++eUnit) + { + const CvUnitInfo& kUnit = GC.getUnitInfo(eUnit); + + for (int iYield = 0; iYield < NUM_YIELD_TYPES; ++iYield) + { + int iCost = kUnit.getYieldCost(iYield); + // Yield costs should never be negative + WTP_ASSERT_MSG(iCost >= 0, + "Unit has negative yield cost"); + } + } +} + +// ============================================================================ +// Profession Equipment Yields +// ============================================================================ + +WTP_TEST(XMLIntegrity, ProfessionEquipmentYieldsValid) +{ + for (ProfessionTypes eProfession = FIRST_PROFESSION; eProfession < NUM_PROFESSION_TYPES; ++eProfession) + { + const CvProfessionInfo& kProfession = GC.getProfessionInfo(eProfession); + + for (int iYield = 0; iYield < NUM_YIELD_TYPES; ++iYield) + { + int iAmount = kProfession.getYieldEquipmentAmount(iYield); + // Equipment amounts should be non-negative + WTP_ASSERT_MSG(iAmount >= 0, + "Profession has negative equipment yield amount"); + } + } +} + +// ============================================================================ +// Building Yield Demands Valid +// ============================================================================ + +WTP_TEST(XMLIntegrity, BuildingYieldDemandsValid) +{ + for (BuildingTypes eBuilding = FIRST_BUILDING; eBuilding < NUM_BUILDING_TYPES; ++eBuilding) + { + const CvBuildingInfo& kBuilding = GC.getBuildingInfo(eBuilding); + const InfoArray& kDemands = kBuilding.getYieldDemands(); + + // InfoArray should have valid content (no crash accessing it) + // The InfoArray itself enforces valid types at construction + (void)kDemands; + } +} + +// ============================================================================ +// Bonus Types Valid in Terrain +// ============================================================================ + +WTP_TEST(XMLIntegrity, TerrainBonusesValid) +{ + for (TerrainTypes eTerrain = FIRST_TERRAIN; eTerrain < NUM_TERRAIN_TYPES; ++eTerrain) + { + const CvTerrainInfo& kTerrain = GC.getTerrainInfo(eTerrain); + + for (int iYield = 0; iYield < NUM_YIELD_TYPES; ++iYield) + { + // Accessing yield info for each terrain should not crash + int iValue = kTerrain.getYield(iYield); + (void)iValue; + } + } +} + +// ============================================================================ +// Trade Location Consistency +// ============================================================================ + +WTP_TEST(XMLIntegrity, TradeLocationSentinelConsistency) +{ + // Verify EUROPE sentinel is valid for isOffMapTradeLocation + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::EUROPE_CITY_ID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::AFRICA_CITY_ID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(CvTradeRoute::PORT_ROYAL_CITY_ID)); + + // Sentinels must not collide with valid city IDs (which start at 0) + WTP_ASSERT(CvTradeRoute::EUROPE_CITY_ID < 0); + WTP_ASSERT(CvTradeRoute::AFRICA_CITY_ID < 0); + WTP_ASSERT(CvTradeRoute::PORT_ROYAL_CITY_ID < 0); +} + +// ============================================================================ +// Test Runner +// ============================================================================ + +void RunXMLIntegrityTests() +{ + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(XMLIntegrity, ProfessionYieldsValid); + WTP_RUN_TEST(XMLIntegrity, BuildingClassValid); + WTP_RUN_TEST(XMLIntegrity, UnitDefaultProfessionValid); + WTP_RUN_TEST(XMLIntegrity, FatherCivEffectValid); + WTP_RUN_TEST(XMLIntegrity, FatherCategoryValid); + WTP_RUN_TEST(XMLIntegrity, UnitYieldCostsNonNegative); + WTP_RUN_TEST(XMLIntegrity, ProfessionEquipmentYieldsValid); + WTP_RUN_TEST(XMLIntegrity, BuildingYieldDemandsValid); + WTP_RUN_TEST(XMLIntegrity, TerrainBonusesValid); + WTP_RUN_TEST(XMLIntegrity, TradeLocationSentinelConsistency); + WTP_RUN_SUITE_END("XMLIntegrity"); +} + +#endif diff --git a/Project Files/DLLSources/WTPTestFramework.cpp b/Project Files/DLLSources/WTPTestFramework.cpp index 946878dcc..c33aa3ec4 100644 --- a/Project Files/DLLSources/WTPTestFramework.cpp +++ b/Project Files/DLLSources/WTPTestFramework.cpp @@ -34,12 +34,14 @@ void WTPTestReport(const char* suiteName, const WTPTestResult& result) // forward declarations for test suites void RunDataStructureTests(); +void RunXMLIntegrityTests(); void RunAllWTPTests() { gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Starting ==="); RunDataStructureTests(); + RunXMLIntegrityTests(); gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Complete ==="); } diff --git a/Project Files/Makefile b/Project Files/Makefile index 5d8bf5b9b..8081b6b5e 100644 --- a/Project Files/Makefile +++ b/Project Files/Makefile @@ -361,6 +361,8 @@ test_DllExport: $(HIDE_CHAR)$(PERL) bin\DllExport.pl "$(SOURCE_DIR)" $(HIDE_CHAR)$(PERL) bin\xml_validation.pl $(HIDE_CHAR)$(PERL) bin\CPP_line_checker.pl + $(HIDE_CHAR)$(PERL) bin\test_determinism.pl + $(HIDE_CHAR)$(PERL) bin\test_text_keys.pl test_Python: header_building $(HIDE_CHAR)$(PERL) bin\python_enum_check.pl diff --git a/Project Files/RaR.vcxproj b/Project Files/RaR.vcxproj index fba988e60..624b5e87b 100644 --- a/Project Files/RaR.vcxproj +++ b/Project Files/RaR.vcxproj @@ -121,6 +121,7 @@ + diff --git a/Project Files/bin/test_determinism.pl b/Project Files/bin/test_determinism.pl new file mode 100644 index 000000000..30dce214b --- /dev/null +++ b/Project Files/bin/test_determinism.pl @@ -0,0 +1,91 @@ +#!/usr/bin/perl -w + +# +# Build-time test: detect non-deterministic function calls in game logic. +# Calls to rand(), srand(), time(), GetTickCount() outside of allowed files +# will cause multiplayer OOS (out-of-sync) bugs. +# +# Allowed files: +# CvRandom.cpp - the synced RNG wrapper (uses srand internally) +# CvCity.cpp - has a deterministic srand-based helper (seeded by coordinates) +# + +use strict; +use warnings; + +my $dir = "DLLSources"; +my $errors = 0; + +# Files allowed to use these functions +my %allowed_rand = ( + "CvRandom.cpp" => 1, + "CvCity.cpp" => 1, +); + +my %allowed_time = (); + +opendir(DIR, $dir) or die "Cannot open $dir: $!\n"; + +while (my $file = readdir(DIR)) +{ + next unless (-f "$dir/$file"); + next unless ($file =~ m/\.cpp$/) or ($file =~ m/\.h$/); + + # Skip test files + next if ($file =~ m/^Test/); + next if ($file =~ m/^WTPTest/); + + open my $fh, '<', "$dir/$file" or die "Cannot open $dir/$file: $!\n"; + my $linenum = 0; + + while (my $line = <$fh>) + { + $linenum++; + + # Skip comments (simple heuristic: lines starting with // or inside /* */) + next if $line =~ m{^\s*//}; + # Skip lines that are commented out + next if $line =~ m{^\s*//.*\brand\b}; + + # Check for rand()/srand() outside allowed files + if (!$allowed_rand{$file}) + { + # Match standalone rand() or srand() calls, not CvRandom or other identifiers + if ($line =~ m/\b(?:std::)?(?:s?rand)\s*\(/ && $line !~ m{^\s*//}) + { + print STDERR "$dir/$file($linenum): Non-deterministic: use of rand()/srand() outside CvRandom\n"; + $errors++; + } + } + + # Check for time()/GetTickCount() in any game logic file + if (!$allowed_time{$file}) + { + if ($line =~ m/\btime\s*\(\s*NULL\s*\)/ && $line !~ m{^\s*//}) + { + print STDERR "$dir/$file($linenum): Non-deterministic: use of time(NULL)\n"; + $errors++; + } + if ($line =~ m/\bGetTickCount\s*\(/ && $line !~ m{^\s*//}) + { + # GetTickCount is OK in profiling code (Profile.cpp etc.) + next if $file eq "Profile.cpp"; + next if $file =~ m/^Cv.*Profile/; + print STDERR "$dir/$file($linenum): Non-deterministic: use of GetTickCount()\n"; + $errors++; + } + } + } + + close $fh; +} + +closedir(DIR); + +if ($errors > 0) +{ + # Report as warnings, not fatal errors — existing code has known uses + print STDERR "test_determinism: $errors potential non-deterministic call(s) found (review for OOS risk)\n"; +} + +exit(0); diff --git a/Project Files/bin/test_text_keys.pl b/Project Files/bin/test_text_keys.pl new file mode 100644 index 000000000..29209f0f2 --- /dev/null +++ b/Project Files/bin/test_text_keys.pl @@ -0,0 +1,106 @@ +#!/usr/bin/perl -w + +# +# Build-time test: verify that TXT_KEY references in XML data files +# have corresponding definitions in Assets/XML/Text/ files. +# +# Scans all CIV4*Infos.xml files for TXT_KEY_* references, +# then checks that each key exists in at least one text XML file. +# + +use strict; +use warnings; +use File::Find; + +my $xml_base = "../Assets/XML/"; +my $text_dir = $xml_base . "Text/"; + +# Step 1: Build a set of all defined TXT_KEY values from text files +my %defined_keys; + +opendir(DIR, $text_dir) or die "Cannot open $text_dir: $!\n"; +while (my $file = readdir(DIR)) +{ + next unless $file =~ m/\.xml$/i; + my $path = $text_dir . $file; + + open my $fh, '<', $path or die "Cannot open $path: $!\n"; + while (my $line = <$fh>) + { + # Match TXT_KEY_... + while ($line =~ m/\s*(TXT_KEY_[A-Z0-9_]+)\s*<\/Tag>/g) + { + $defined_keys{$1} = $path; + } + } + close $fh; +} +closedir(DIR); + +my $num_defined = scalar keys %defined_keys; +if ($num_defined == 0) +{ + die "test_text_keys: Found no TXT_KEY definitions in $text_dir — check path\n"; +} + +# Step 2: Scan data XML files for TXT_KEY references +my @data_dirs = ( + "Buildings/", "Units/", "GameInfo/", "Civilizations/", + "Terrain/", "Events/", "BasicInfos/", "CivEffects/", + "Interface/", "Misc/", "Art/", +); + +my %missing_keys; +my $refs_checked = 0; + +for my $subdir (@data_dirs) +{ + my $dir = $xml_base . $subdir; + next unless -d $dir; + + opendir(my $dh, $dir) or next; + while (my $file = readdir($dh)) + { + next unless $file =~ m/\.xml$/i; + # Skip schema files + next if $file =~ m/Schema/i; + + my $path = $dir . $file; + open my $fh, '<', $path or next; + my $linenum = 0; + + while (my $line = <$fh>) + { + $linenum++; + + # Match TXT_KEY references in element content (not inside elements) + while ($line =~ m/>(TXT_KEY_[A-Z0-9_]+) 0) +{ + my $count = scalar keys %missing_keys; + print STDERR "test_text_keys: $count TXT_KEY reference(s) missing from text XML files:\n"; + for my $key (sort keys %missing_keys) + { + print STDERR " $key (referenced in $missing_keys{$key})\n"; + } + # Warning only — some keys may be defined in code or other locations + print STDERR "test_text_keys: Review the above keys. Some may be generated at runtime.\n"; +} + +exit(0); From d703d0f22daad75e717ffeb4457209e5836927cd Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 21:11:52 -0700 Subject: [PATCH 04/13] Add savegame round-trip tests (Phase 3) and fix test failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3: MemoryStream (in-memory FDataStreamBase) with 23 round-trip tests covering primitives, strings, IDInfo, JustInTimeArray, and enum types. Fix XMLIntegrity test: allow NO_YIELD (-1) in profession yield lists, as the game code explicitly checks and skips it. Fix int-to-enum casts for VC++ 2003 strict type checking. Fix Savegame test: JustInTimeArray::Read() always allocates, even for empty arrays — removed incorrect !isAllocated() assertion. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/TestSavegame.cpp | 660 ++++++++++++++++++ Project Files/DLLSources/TestXMLIntegrity.cpp | 14 +- Project Files/DLLSources/WTPTestFramework.cpp | 2 + Project Files/RaR.vcxproj | 1 + 4 files changed, 671 insertions(+), 6 deletions(-) create mode 100644 Project Files/DLLSources/TestSavegame.cpp diff --git a/Project Files/DLLSources/TestSavegame.cpp b/Project Files/DLLSources/TestSavegame.cpp new file mode 100644 index 000000000..64a2fe8f0 --- /dev/null +++ b/Project Files/DLLSources/TestSavegame.cpp @@ -0,0 +1,660 @@ + +#include "CvGameCoreDLL.h" +#include "WTPTestFramework.h" + +#ifdef FASSERT_ENABLE + +#include "CvTradeRoute.h" + +// ============================================================================ +// MemoryStream — minimal FDataStreamBase for testing +// ============================================================================ + +class MemoryStream : public FDataStreamBase +{ +public: + MemoryStream() : m_iPos(0) {} + + // -- Position/State -- + void Rewind() { m_iPos = 0; } + bool AtEnd() { return m_iPos >= m_data.size(); } + void FastFwd() { m_iPos = m_data.size(); } + unsigned int GetPosition() const { return m_iPos; } + void SetPosition(unsigned int position) { m_iPos = position; } + unsigned int GetEOF() const { return m_data.size(); } + unsigned int GetSizeLeft() const { return m_data.size() - m_iPos; } + void Truncate() { m_data.resize(m_iPos); } + void Flush() {} + void CopyToMem(void* mem) { memcpy(mem, &m_data[0], m_data.size()); } + + // -- Raw read/write helpers -- + void rawRead(void* dest, unsigned int bytes) + { + FAssert(m_iPos + bytes <= m_data.size()); + memcpy(dest, &m_data[m_iPos], bytes); + m_iPos += bytes; + } + + void rawWrite(const void* src, unsigned int bytes) + { + if (m_iPos + bytes > m_data.size()) + m_data.resize(m_iPos + bytes); + memcpy(&m_data[m_iPos], src, bytes); + m_iPos += bytes; + } + + // -- String Write -- + unsigned int WriteString(const wchar* szName) + { + unsigned int len = szName ? (unsigned int)wcslen(szName) : 0; + Write(len); + if (len > 0) Write((int)len, (const short*)szName); + return len; + } + unsigned int WriteString(const char* szName) + { + unsigned int len = szName ? (unsigned int)strlen(szName) : 0; + Write(len); + if (len > 0) Write((int)len, szName); + return len; + } + unsigned int WriteString(const std::string& s) { return WriteString(s.c_str()); } + unsigned int WriteString(const std::wstring& s) { return WriteString(s.c_str()); } + unsigned int WriteString(int count, std::string values[]) + { + for (int i = 0; i < count; ++i) WriteString(values[i]); + return 0; + } + unsigned int WriteString(int count, std::wstring values[]) + { + for (int i = 0; i < count; ++i) WriteString(values[i]); + return 0; + } + + // -- String Read -- + unsigned int ReadString(char* szName) + { + unsigned int len; Read(&len); + if (len > 0) Read((int)len, szName); + szName[len] = 0; + return len; + } + unsigned int ReadString(wchar* szName) + { + unsigned int len; Read(&len); + if (len > 0) Read((int)len, (short*)szName); + szName[len] = 0; + return len; + } + unsigned int ReadString(std::string& s) + { + unsigned int len; Read(&len); + s.resize(len); + if (len > 0) Read((int)len, &s[0]); + return len; + } + unsigned int ReadString(std::wstring& s) + { + unsigned int len; Read(&len); + s.resize(len); + if (len > 0) Read((int)len, (short*)&s[0]); + return len; + } + unsigned int ReadString(int count, std::string values[]) + { + for (int i = 0; i < count; ++i) ReadString(values[i]); + return 0; + } + unsigned int ReadString(int count, std::wstring values[]) + { + for (int i = 0; i < count; ++i) ReadString(values[i]); + return 0; + } + char* ReadString() + { + unsigned int len; Read(&len); + char* buf = new char[len + 1]; + if (len > 0) Read((int)len, buf); + buf[len] = 0; + return buf; + } + wchar* ReadWideString() + { + unsigned int len; Read(&len); + wchar* buf = new wchar[len + 1]; + if (len > 0) Read((int)len, (short*)buf); + buf[len] = 0; + return buf; + } + + // -- Primitive Read (single) -- + void Read(char* v) { rawRead(v, sizeof(char)); } + void Read(byte* v) { rawRead(v, sizeof(byte)); } + void Read(bool* v) { rawRead(v, sizeof(bool)); } + void Read(short* v) { rawRead(v, sizeof(short)); } + void Read(unsigned short* v) { rawRead(v, sizeof(unsigned short)); } + void Read(int* v) { rawRead(v, sizeof(int)); } + void Read(unsigned int* v) { rawRead(v, sizeof(unsigned int)); } + void Read(long* v) { rawRead(v, sizeof(long)); } + void Read(unsigned long* v) { rawRead(v, sizeof(unsigned long)); } + void Read(float* v) { rawRead(v, sizeof(float)); } + void Read(double* v) { rawRead(v, sizeof(double)); } + + // -- Primitive Read (array) -- + void Read(int count, char values[]) { rawRead(values, count * sizeof(char)); } + void Read(int count, byte values[]) { rawRead(values, count * sizeof(byte)); } + void Read(int count, bool values[]) { rawRead(values, count * sizeof(bool)); } + void Read(int count, short values[]) { rawRead(values, count * sizeof(short)); } + void Read(int count, unsigned short values[]) { rawRead(values, count * sizeof(unsigned short)); } + void Read(int count, int values[]) { rawRead(values, count * sizeof(int)); } + void Read(int count, unsigned int values[]) { rawRead(values, count * sizeof(unsigned int)); } + void Read(int count, long values[]) { rawRead(values, count * sizeof(long)); } + void Read(int count, unsigned long values[]) { rawRead(values, count * sizeof(unsigned long)); } + void Read(int count, float values[]) { rawRead(values, count * sizeof(float)); } + void Read(int count, double values[]) { rawRead(values, count * sizeof(double)); } + + // -- Primitive Write (single) -- + void Write(char v) { rawWrite(&v, sizeof(char)); } + void Write(byte v) { rawWrite(&v, sizeof(byte)); } + void Write(bool v) { rawWrite(&v, sizeof(bool)); } + void Write(short v) { rawWrite(&v, sizeof(short)); } + void Write(unsigned short v) { rawWrite(&v, sizeof(unsigned short)); } + void Write(int v) { rawWrite(&v, sizeof(int)); } + void Write(unsigned int v) { rawWrite(&v, sizeof(unsigned int)); } + void Write(long v) { rawWrite(&v, sizeof(long)); } + void Write(unsigned long v) { rawWrite(&v, sizeof(unsigned long)); } + void Write(float v) { rawWrite(&v, sizeof(float)); } + void Write(double v) { rawWrite(&v, sizeof(double)); } + + // -- Primitive Write (array) -- + void Write(int count, const char values[]) { rawWrite(values, count * sizeof(char)); } + void Write(int count, const byte values[]) { rawWrite(values, count * sizeof(byte)); } + void Write(int count, const bool values[]) { rawWrite(values, count * sizeof(bool)); } + void Write(int count, const short values[]) { rawWrite(values, count * sizeof(short)); } + void Write(int count, const unsigned short values[]) { rawWrite(values, count * sizeof(unsigned short)); } + void Write(int count, const int values[]) { rawWrite(values, count * sizeof(int)); } + void Write(int count, const unsigned int values[]) { rawWrite(values, count * sizeof(unsigned int)); } + void Write(int count, const long values[]) { rawWrite(values, count * sizeof(long)); } + void Write(int count, const unsigned long values[]) { rawWrite(values, count * sizeof(unsigned long)); } + void Write(int count, const float values[]) { rawWrite(values, count * sizeof(float)); } + void Write(int count, const double values[]) { rawWrite(values, count * sizeof(double)); } + +private: + std::vector m_data; + unsigned int m_iPos; +}; + +// ============================================================================ +// Primitive Round-Trip Tests +// ============================================================================ + +WTP_TEST(Savegame, IntRoundTrip) +{ + MemoryStream stream; + int iWrite = 42; + stream.Write(iWrite); + + stream.Rewind(); + int iRead = 0; + stream.Read(&iRead); + WTP_ASSERT_EQ(42, iRead); +} + +WTP_TEST(Savegame, BoolRoundTrip) +{ + MemoryStream stream; + stream.Write(true); + stream.Write(false); + + stream.Rewind(); + bool b1 = false, b2 = true; + stream.Read(&b1); + stream.Read(&b2); + WTP_ASSERT(b1 == true); + WTP_ASSERT(b2 == false); +} + +WTP_TEST(Savegame, ShortRoundTrip) +{ + MemoryStream stream; + short sWrite = -12345; + stream.Write(sWrite); + + stream.Rewind(); + short sRead = 0; + stream.Read(&sRead); + WTP_ASSERT_EQ(-12345, static_cast(sRead)); +} + +WTP_TEST(Savegame, UnsignedShortRoundTrip) +{ + MemoryStream stream; + unsigned short usWrite = 65535; + stream.Write(usWrite); + + stream.Rewind(); + unsigned short usRead = 0; + stream.Read(&usRead); + WTP_ASSERT_EQ(65535, static_cast(usRead)); +} + +WTP_TEST(Savegame, FloatRoundTrip) +{ + MemoryStream stream; + float fWrite = 3.14f; + stream.Write(fWrite); + + stream.Rewind(); + float fRead = 0.0f; + stream.Read(&fRead); + // Float comparison: check within tolerance + WTP_ASSERT_MSG(fRead > 3.13f && fRead < 3.15f, "float round-trip mismatch"); +} + +WTP_TEST(Savegame, DoubleRoundTrip) +{ + MemoryStream stream; + double dWrite = 2.718281828; + stream.Write(dWrite); + + stream.Rewind(); + double dRead = 0.0; + stream.Read(&dRead); + WTP_ASSERT_MSG(dRead > 2.718 && dRead < 2.719, "double round-trip mismatch"); +} + +WTP_TEST(Savegame, MultipleValuesSequential) +{ + MemoryStream stream; + stream.Write(100); + stream.Write(static_cast(200)); + stream.Write(true); + stream.Write(static_cast(255)); + + stream.Rewind(); + int i; short s; bool b; byte by; + stream.Read(&i); + stream.Read(&s); + stream.Read(&b); + stream.Read(&by); + WTP_ASSERT_EQ(100, i); + WTP_ASSERT_EQ(200, static_cast(s)); + WTP_ASSERT(b == true); + WTP_ASSERT_EQ(255, static_cast(by)); +} + +WTP_TEST(Savegame, IntArrayRoundTrip) +{ + MemoryStream stream; + int arrWrite[3] = {10, 20, 30}; + stream.Write(3, arrWrite); + + stream.Rewind(); + int arrRead[3] = {0, 0, 0}; + stream.Read(3, arrRead); + WTP_ASSERT_EQ(10, arrRead[0]); + WTP_ASSERT_EQ(20, arrRead[1]); + WTP_ASSERT_EQ(30, arrRead[2]); +} + +// ============================================================================ +// Stream State Tests +// ============================================================================ + +WTP_TEST(Savegame, StreamPosition) +{ + MemoryStream stream; + WTP_ASSERT_EQ(0, static_cast(stream.GetPosition())); + WTP_ASSERT_EQ(0, static_cast(stream.GetEOF())); + + stream.Write(42); + WTP_ASSERT_EQ(4, static_cast(stream.GetPosition())); + WTP_ASSERT_EQ(4, static_cast(stream.GetEOF())); + + stream.Rewind(); + WTP_ASSERT_EQ(0, static_cast(stream.GetPosition())); + WTP_ASSERT_EQ(4, static_cast(stream.GetEOF())); + WTP_ASSERT_EQ(4, static_cast(stream.GetSizeLeft())); +} + +WTP_TEST(Savegame, StreamAtEnd) +{ + MemoryStream stream; + WTP_ASSERT(stream.AtEnd()); + + stream.Write(1); + WTP_ASSERT(stream.AtEnd()); // write position is at end + + stream.Rewind(); + WTP_ASSERT(!stream.AtEnd()); + + int dummy; + stream.Read(&dummy); + WTP_ASSERT(stream.AtEnd()); +} + +WTP_TEST(Savegame, StreamSetPosition) +{ + MemoryStream stream; + stream.Write(10); + stream.Write(20); + stream.Write(30); + + // Read second value by seeking + stream.SetPosition(4); // skip first int + int val; + stream.Read(&val); + WTP_ASSERT_EQ(20, val); + + // Seek back to first + stream.SetPosition(0); + stream.Read(&val); + WTP_ASSERT_EQ(10, val); +} + +// ============================================================================ +// String Round-Trip Tests +// ============================================================================ + +WTP_TEST(Savegame, StringRoundTrip) +{ + MemoryStream stream; + stream.WriteString("hello"); + + stream.Rewind(); + char buf[32]; + stream.ReadString(buf); + WTP_ASSERT_MSG(strcmp(buf, "hello") == 0, "string round-trip mismatch"); +} + +WTP_TEST(Savegame, StdStringRoundTrip) +{ + MemoryStream stream; + std::string sWrite = "test_string"; + stream.WriteString(sWrite); + + stream.Rewind(); + std::string sRead; + stream.ReadString(sRead); + WTP_ASSERT_MSG(sRead == "test_string", "std::string round-trip mismatch"); +} + +WTP_TEST(Savegame, EmptyStringRoundTrip) +{ + MemoryStream stream; + stream.WriteString(""); + + stream.Rewind(); + std::string sRead = "notempty"; + stream.ReadString(sRead); + WTP_ASSERT_MSG(sRead.empty(), "empty string round-trip should produce empty"); +} + +// ============================================================================ +// IDInfo Round-Trip Tests +// ============================================================================ + +WTP_TEST(Savegame, IDInfoDefaultRoundTrip) +{ + MemoryStream stream; + IDInfo original; + original.write(&stream); + + stream.Rewind(); + IDInfo loaded; + loaded.eOwner = FIRST_PLAYER; // set non-default to verify read works + loaded.iID = 999; + loaded.read(&stream); + + WTP_ASSERT_EQ(NO_PLAYER, loaded.eOwner); + WTP_ASSERT_EQ(-1, loaded.iID); +} + +WTP_TEST(Savegame, IDInfoWithDataRoundTrip) +{ + MemoryStream stream; + IDInfo original(FIRST_PLAYER, 42); + original.write(&stream); + + stream.Rewind(); + IDInfo loaded; + loaded.read(&stream); + + WTP_ASSERT(loaded == original); + WTP_ASSERT_EQ(FIRST_PLAYER, loaded.eOwner); + WTP_ASSERT_EQ(42, loaded.iID); +} + +WTP_TEST(Savegame, IDInfoTradeRouteSentinelRoundTrip) +{ + // Test that off-map sentinel IDs survive serialization + MemoryStream stream; + + IDInfo europeInfo(FIRST_PLAYER, CvTradeRoute::EUROPE_CITY_ID); + IDInfo africaInfo(FIRST_PLAYER, CvTradeRoute::AFRICA_CITY_ID); + IDInfo portRoyalInfo(FIRST_PLAYER, CvTradeRoute::PORT_ROYAL_CITY_ID); + + europeInfo.write(&stream); + africaInfo.write(&stream); + portRoyalInfo.write(&stream); + + stream.Rewind(); + + IDInfo loadedEurope, loadedAfrica, loadedPortRoyal; + loadedEurope.read(&stream); + loadedAfrica.read(&stream); + loadedPortRoyal.read(&stream); + + WTP_ASSERT(loadedEurope == europeInfo); + WTP_ASSERT(loadedAfrica == africaInfo); + WTP_ASSERT(loadedPortRoyal == portRoyalInfo); + + // Verify sentinels are correctly recognized after round-trip + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(loadedEurope.iID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(loadedAfrica.iID)); + WTP_ASSERT(CvTradeRoute::isOffMapTradeLocation(loadedPortRoyal.iID)); +} + +// ============================================================================ +// JustInTimeArray Round-Trip Tests +// ============================================================================ + +WTP_TEST(Savegame, JITArrayEmptyRoundTrip) +{ + MemoryStream stream; + JustInTimeArray arrWrite(JIT_ARRAY_ROUTE); + arrWrite.Write(&stream); + + stream.Rewind(); + JustInTimeArray arrRead(JIT_ARRAY_ROUTE); + arrRead.Read(&stream); + + // Note: JustInTimeArray::Read() always calls allocate(), even for empty arrays + // So after round-trip, the array is allocated but all values are default (0) + WTP_ASSERT_EQ(0, arrRead.get(0)); +} + +WTP_TEST(Savegame, JITArrayWithDataRoundTrip) +{ + MemoryStream stream; + JustInTimeArray arrWrite(JIT_ARRAY_ROUTE); + arrWrite.set(42, 0); + if (arrWrite.length() > 1) + { + arrWrite.set(99, 1); + } + arrWrite.Write(&stream); + + stream.Rewind(); + JustInTimeArray arrRead(JIT_ARRAY_ROUTE); + arrRead.Read(&stream); + + WTP_ASSERT(arrRead.isAllocated()); + WTP_ASSERT_EQ(42, arrRead.get(0)); + if (arrRead.length() > 1) + { + WTP_ASSERT_EQ(99, arrRead.get(1)); + } +} + +WTP_TEST(Savegame, JITArrayResetAndRoundTrip) +{ + MemoryStream stream; + JustInTimeArray arrWrite(JIT_ARRAY_ROUTE); + arrWrite.set(10, 0); + arrWrite.reset(); + // Write an empty (reset) array + arrWrite.Write(&stream); + + stream.Rewind(); + JustInTimeArray arrRead(JIT_ARRAY_ROUTE); + arrRead.set(777, 0); // pre-populate to verify reset works + arrRead.Read(&stream); + + WTP_ASSERT_EQ(0, arrRead.get(0)); +} + +// ============================================================================ +// Enum Round-Trip via FDataStreamBase Template +// ============================================================================ + +WTP_TEST(Savegame, EnumRoundTrip) +{ + MemoryStream stream; + YieldTypes eWrite = YIELD_FOOD; + stream.Write(static_cast(eWrite)); + + stream.Rewind(); + YieldTypes eRead = NO_YIELD; + int iRead = 0; + stream.Read(&iRead); + eRead = static_cast(iRead); + WTP_ASSERT_EQ(YIELD_FOOD, eRead); +} + +WTP_TEST(Savegame, EnumNegativeRoundTrip) +{ + MemoryStream stream; + YieldTypes eWrite = NO_YIELD; + stream.Write(static_cast(eWrite)); + + stream.Rewind(); + YieldTypes eRead = YIELD_FOOD; + int iRead = 0; + stream.Read(&iRead); + eRead = static_cast(iRead); + WTP_ASSERT_EQ(NO_YIELD, eRead); +} + +// ============================================================================ +// Edge Cases +// ============================================================================ + +WTP_TEST(Savegame, LargeIntValues) +{ + MemoryStream stream; + int iMax = 2147483647; // MAX_INT + int iMin = -2147483647 - 1; // MIN_INT + stream.Write(iMax); + stream.Write(iMin); + + stream.Rewind(); + int iReadMax, iReadMin; + stream.Read(&iReadMax); + stream.Read(&iReadMin); + WTP_ASSERT_EQ(iMax, iReadMax); + WTP_ASSERT_EQ(iMin, iReadMin); +} + +WTP_TEST(Savegame, ZeroLengthArrayRoundTrip) +{ + MemoryStream stream; + // Write zero-count array (should write nothing) + int dummy[1] = {0}; + stream.Write(0, dummy); + + WTP_ASSERT_EQ(0, static_cast(stream.GetEOF())); +} + +WTP_TEST(Savegame, TruncateStream) +{ + MemoryStream stream; + stream.Write(10); + stream.Write(20); + stream.Write(30); + WTP_ASSERT_EQ(12, static_cast(stream.GetEOF())); + + stream.SetPosition(8); // after second int + stream.Truncate(); + WTP_ASSERT_EQ(8, static_cast(stream.GetEOF())); + + stream.Rewind(); + int v1, v2; + stream.Read(&v1); + stream.Read(&v2); + WTP_ASSERT_EQ(10, v1); + WTP_ASSERT_EQ(20, v2); + WTP_ASSERT(stream.AtEnd()); +} + +// ============================================================================ +// Test Runner +// ============================================================================ + +void RunSavegameTests() +{ + // Primitive round-trips + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, IntRoundTrip); + WTP_RUN_TEST(Savegame, BoolRoundTrip); + WTP_RUN_TEST(Savegame, ShortRoundTrip); + WTP_RUN_TEST(Savegame, UnsignedShortRoundTrip); + WTP_RUN_TEST(Savegame, FloatRoundTrip); + WTP_RUN_TEST(Savegame, DoubleRoundTrip); + WTP_RUN_TEST(Savegame, MultipleValuesSequential); + WTP_RUN_TEST(Savegame, IntArrayRoundTrip); + WTP_RUN_SUITE_END("Savegame_Primitives"); + + // Stream state + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, StreamPosition); + WTP_RUN_TEST(Savegame, StreamAtEnd); + WTP_RUN_TEST(Savegame, StreamSetPosition); + WTP_RUN_SUITE_END("Savegame_StreamState"); + + // String round-trips + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, StringRoundTrip); + WTP_RUN_TEST(Savegame, StdStringRoundTrip); + WTP_RUN_TEST(Savegame, EmptyStringRoundTrip); + WTP_RUN_SUITE_END("Savegame_Strings"); + + // IDInfo round-trips + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, IDInfoDefaultRoundTrip); + WTP_RUN_TEST(Savegame, IDInfoWithDataRoundTrip); + WTP_RUN_TEST(Savegame, IDInfoTradeRouteSentinelRoundTrip); + WTP_RUN_SUITE_END("Savegame_IDInfo"); + + // JustInTimeArray round-trips + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, JITArrayEmptyRoundTrip); + WTP_RUN_TEST(Savegame, JITArrayWithDataRoundTrip); + WTP_RUN_TEST(Savegame, JITArrayResetAndRoundTrip); + WTP_RUN_SUITE_END("Savegame_JustInTimeArray"); + + // Enum round-trips + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, EnumRoundTrip); + WTP_RUN_TEST(Savegame, EnumNegativeRoundTrip); + WTP_RUN_SUITE_END("Savegame_Enums"); + + // Edge cases + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Savegame, LargeIntValues); + WTP_RUN_TEST(Savegame, ZeroLengthArrayRoundTrip); + WTP_RUN_TEST(Savegame, TruncateStream); + WTP_RUN_SUITE_END("Savegame_EdgeCases"); +} + +#endif diff --git a/Project Files/DLLSources/TestXMLIntegrity.cpp b/Project Files/DLLSources/TestXMLIntegrity.cpp index f1316923a..4e962ddfd 100644 --- a/Project Files/DLLSources/TestXMLIntegrity.cpp +++ b/Project Files/DLLSources/TestXMLIntegrity.cpp @@ -19,14 +19,16 @@ WTP_TEST(XMLIntegrity, ProfessionYieldsValid) for (int i = 0; i < kProfession.getNumYieldsProduced(); ++i) { int eYield = kProfession.getYieldsProduced(i); - WTP_ASSERT_MSG(eYield >= 0 && eYield < NUM_YIELD_TYPES, + // NO_YIELD (-1) is valid — game code checks and skips it + WTP_ASSERT_MSG(eYield == NO_YIELD || (eYield >= 0 && eYield < NUM_YIELD_TYPES), "Profession has invalid produced yield"); } for (int i = 0; i < kProfession.getNumYieldsConsumed(); ++i) { int eYield = kProfession.getYieldsConsumed(i); - WTP_ASSERT_MSG(eYield >= 0 && eYield < NUM_YIELD_TYPES, + // NO_YIELD (-1) is valid — game code checks and skips it + WTP_ASSERT_MSG(eYield == NO_YIELD || (eYield >= 0 && eYield < NUM_YIELD_TYPES), "Profession has invalid consumed yield"); } } @@ -113,9 +115,9 @@ WTP_TEST(XMLIntegrity, UnitYieldCostsNonNegative) { const CvUnitInfo& kUnit = GC.getUnitInfo(eUnit); - for (int iYield = 0; iYield < NUM_YIELD_TYPES; ++iYield) + for (YieldTypes eYield = FIRST_YIELD; eYield < NUM_YIELD_TYPES; ++eYield) { - int iCost = kUnit.getYieldCost(iYield); + int iCost = kUnit.getYieldCost(eYield); // Yield costs should never be negative WTP_ASSERT_MSG(iCost >= 0, "Unit has negative yield cost"); @@ -170,10 +172,10 @@ WTP_TEST(XMLIntegrity, TerrainBonusesValid) { const CvTerrainInfo& kTerrain = GC.getTerrainInfo(eTerrain); - for (int iYield = 0; iYield < NUM_YIELD_TYPES; ++iYield) + for (YieldTypes eYield = FIRST_YIELD; eYield < NUM_YIELD_TYPES; ++eYield) { // Accessing yield info for each terrain should not crash - int iValue = kTerrain.getYield(iYield); + int iValue = kTerrain.getYield(eYield); (void)iValue; } } diff --git a/Project Files/DLLSources/WTPTestFramework.cpp b/Project Files/DLLSources/WTPTestFramework.cpp index c33aa3ec4..6b2b87e6c 100644 --- a/Project Files/DLLSources/WTPTestFramework.cpp +++ b/Project Files/DLLSources/WTPTestFramework.cpp @@ -35,6 +35,7 @@ void WTPTestReport(const char* suiteName, const WTPTestResult& result) // forward declarations for test suites void RunDataStructureTests(); void RunXMLIntegrityTests(); +void RunSavegameTests(); void RunAllWTPTests() { @@ -42,6 +43,7 @@ void RunAllWTPTests() RunDataStructureTests(); RunXMLIntegrityTests(); + RunSavegameTests(); gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Complete ==="); } diff --git a/Project Files/RaR.vcxproj b/Project Files/RaR.vcxproj index 624b5e87b..556758631 100644 --- a/Project Files/RaR.vcxproj +++ b/Project Files/RaR.vcxproj @@ -122,6 +122,7 @@ + From 069009b2423888a71e509fce5ee3bd199b0ffe7e Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 21:12:02 -0700 Subject: [PATCH 05/13] Add optional GAME_MODS_PATH for auto-installing mod after build Set GAME_MODS_PATH in Makefile.settings to automatically xcopy the mod to the game's Mods directory as a post-build step. Co-Authored-By: Claude Opus 4.6 --- Project Files/Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Project Files/Makefile b/Project Files/Makefile index 8081b6b5e..454b21fc5 100644 --- a/Project Files/Makefile +++ b/Project Files/Makefile @@ -284,6 +284,11 @@ build: $(Target_BIN) -xcopy "..\PrivateMaps\*" "$(YOURMOD)\PrivateMaps\*" /e /y /f /d !ENDIF !ENDIF +!IFDEF GAME_MODS_PATH + @ECHO Installing mod to $(GAME_MODS_PATH) + -xcopy "$(YOURMOD)\Assets\*" "$(GAME_MODS_PATH)\Assets\*" /e /y /d /q + -xcopy "$(YOURMOD)\PrivateMaps\*" "$(GAME_MODS_PATH)\PrivateMaps\*" /e /y /d /q +!ENDIF precompile: Target_unfinished $(Target_PCH) From f14154bda7b5c948bba84391f9182ef47fb12b83 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 23:21:20 -0700 Subject: [PATCH 06/13] Fix trade route automation for Africa and Port Royal destinations Ships with automated trade routes to off-map destinations (Africa, Port Royal) would get stuck upon arrival or lose their route assignments when crossing the ocean. Four root causes fixed: - AI_update() now intercepts units in Port Royal (was only Europe/Africa) - AI_europeUpdate() handles AUTOMATE_TRANSPORT_ROUTES ships: sells cargo and crosses back automatically (previously only AUTOMATE_FULL was handled) - AI_transportMoveRoutes() routes to correct destination via new helper AI_sailToOffMapTradeDestination() instead of always sailing to Europe - setUnitTravelState() preserves automation and trade route assignments across group splits during ocean travel Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/CvSelectionGroup.cpp | 5 ++ Project Files/DLLSources/CvSelectionGroup.h | 1 + .../DLLSources/CvSelectionGroupAI.cpp | 2 +- Project Files/DLLSources/CvUnit.cpp | 19 ++++++ Project Files/DLLSources/CvUnitAI.cpp | 60 ++++++++++++++++++- Project Files/DLLSources/CvUnitAI.h | 1 + 6 files changed, 84 insertions(+), 4 deletions(-) diff --git a/Project Files/DLLSources/CvSelectionGroup.cpp b/Project Files/DLLSources/CvSelectionGroup.cpp index 2deeaa746..1880515eb 100644 --- a/Project Files/DLLSources/CvSelectionGroup.cpp +++ b/Project Files/DLLSources/CvSelectionGroup.cpp @@ -4427,6 +4427,11 @@ void CvSelectionGroup::clearTradeRoutes() gDLL->getInterfaceIFace()->setDirty(Domestic_Advisor_DIRTY_BIT, true); } +const std::set& CvSelectionGroup::getTradeRoutes() const +{ + return m_aTradeRoutes; +} + void CvSelectionGroup::speakWithChief() { CvUnit* pBestUnit = NULL; diff --git a/Project Files/DLLSources/CvSelectionGroup.h b/Project Files/DLLSources/CvSelectionGroup.h index 371122d8b..6162df40c 100644 --- a/Project Files/DLLSources/CvSelectionGroup.h +++ b/Project Files/DLLSources/CvSelectionGroup.h @@ -203,6 +203,7 @@ class CvSelectionGroup void assignTradeRoute(int iRouteID, bool bAssign); bool isAssignedTradeRoute(int iRouteId) const; void clearTradeRoutes(); + const std::set& getTradeRoutes() const; void speakWithChief(); diff --git a/Project Files/DLLSources/CvSelectionGroupAI.cpp b/Project Files/DLLSources/CvSelectionGroupAI.cpp index 7e68e9668..6e9ae20b8 100644 --- a/Project Files/DLLSources/CvSelectionGroupAI.cpp +++ b/Project Files/DLLSources/CvSelectionGroupAI.cpp @@ -252,7 +252,7 @@ bool CvSelectionGroupAI::AI_update() CvUnit* pHeadUnit = getHeadUnit(); if (pHeadUnit != NULL) { - if ((pHeadUnit->getUnitTravelState() == UNIT_TRAVEL_STATE_IN_EUROPE || pHeadUnit->getUnitTravelState() == UNIT_TRAVEL_STATE_IN_AFRICA) && AI_isControlled()) + if ((pHeadUnit->getUnitTravelState() == UNIT_TRAVEL_STATE_IN_EUROPE || pHeadUnit->getUnitTravelState() == UNIT_TRAVEL_STATE_IN_AFRICA || pHeadUnit->getUnitTravelState() == UNIT_TRAVEL_STATE_IN_PORT_ROYAL) && AI_isControlled()) { pEntityNode = headUnitNode(); diff --git a/Project Files/DLLSources/CvUnit.cpp b/Project Files/DLLSources/CvUnit.cpp index 50eaa774d..b0ffd73e7 100644 --- a/Project Files/DLLSources/CvUnit.cpp +++ b/Project Files/DLLSources/CvUnit.cpp @@ -14901,6 +14901,15 @@ void CvUnit::setUnitTravelState(UnitTravelStates eState, bool bShowEuropeScreen) if (getGroup() != NULL) { + // Save automation state before splitting, as splitGroup -> deleteUnitNode + // clears automation and trade routes on the old group + const AutomateTypes eAutomateType = getGroup()->getAutomateType(); + std::set savedTradeRoutes; + if (eAutomateType == AUTOMATE_TRANSPORT_ROUTES) + { + savedTradeRoutes = getGroup()->getTradeRoutes(); + } + if (!isHuman()) { // Erik: Unconditionally separate all units (all units will be re-assigned to a group with the unit as its single member) @@ -14912,6 +14921,16 @@ void CvUnit::setUnitTravelState(UnitTravelStates eState, bool bShowEuropeScreen) if (!getGroup()->getHeadUnit()->isYield()) getGroup()->splitGroup(1, this); } + + // Restore trade route automation on the new group after split + if (eAutomateType == AUTOMATE_TRANSPORT_ROUTES && getGroup() != NULL) + { + getGroup()->setAutomateType(AUTOMATE_TRANSPORT_ROUTES); + for (std::set::const_iterator it = savedTradeRoutes.begin(); it != savedTradeRoutes.end(); ++it) + { + getGroup()->assignTradeRoute(*it, true); + } + } } if (!isOnMapInternal()) diff --git a/Project Files/DLLSources/CvUnitAI.cpp b/Project Files/DLLSources/CvUnitAI.cpp index 054ad0acd..50b8b7fde 100644 --- a/Project Files/DLLSources/CvUnitAI.cpp +++ b/Project Files/DLLSources/CvUnitAI.cpp @@ -23,6 +23,7 @@ #include "CvDLLFAStarIFaceBase.h" #include "CvSavegame.h" +#include "CvTradeRoute.h" #include "BetterBTSAI.h" #define FOUND_RANGE (7) @@ -101,7 +102,7 @@ bool CvUnitAI::AI_update() } } - if (getUnitTravelState() == UNIT_TRAVEL_STATE_IN_EUROPE || getUnitTravelState() == UNIT_TRAVEL_STATE_IN_AFRICA) + if (getUnitTravelState() == UNIT_TRAVEL_STATE_IN_EUROPE || getUnitTravelState() == UNIT_TRAVEL_STATE_IN_AFRICA || getUnitTravelState() == UNIT_TRAVEL_STATE_IN_PORT_ROYAL) { AI_europeUpdate(); return false; @@ -413,6 +414,30 @@ bool CvUnitAI::AI_europeUpdate() return false; } } + + // Handle automated trade route ships at off-map locations: + // sell cargo and cross back to the Americas + if (getGroup()->isAutomated() && (getGroup()->getAutomateType() == AUTOMATE_TRANSPORT_ROUTES)) + { + if (getUnitTravelState() == UNIT_TRAVEL_STATE_IN_EUROPE) + { + AI_sellYieldUnits(TRADE_LOCATION_EUROPE); + crossOcean(UNIT_TRAVEL_STATE_FROM_EUROPE); + return false; + } + if (getUnitTravelState() == UNIT_TRAVEL_STATE_IN_AFRICA) + { + AI_sellYieldUnits(TRADE_LOCATION_AFRICA); + crossOcean(UNIT_TRAVEL_STATE_FROM_AFRICA); + return false; + } + if (getUnitTravelState() == UNIT_TRAVEL_STATE_IN_PORT_ROYAL) + { + AI_sellYieldUnits(TRADE_LOCATION_PORT_ROYAL); + crossOcean(UNIT_TRAVEL_STATE_FROM_PORT_ROYAL); + return false; + } + } } if (!(getGroup()->isAutomated() && (getGroup()->getAutomateType() != AUTOMATE_FULL))) @@ -3969,7 +3994,7 @@ void CvUnitAI::AI_transportMoveRoutes() if (AI_getUnitAIState() == UNITAI_STATE_SAIL) { - if (AI_sailToEurope()) + if (AI_sailToOffMapTradeDestination()) { return; } @@ -3982,7 +4007,7 @@ void CvUnitAI::AI_transportMoveRoutes() if (AI_getUnitAIState() == UNITAI_STATE_SAIL) { - if (AI_sailToEurope()) + if (AI_sailToOffMapTradeDestination()) { return; } @@ -6902,6 +6927,35 @@ bool CvUnitAI::AI_sailToPortRoyal(bool bMove) } // R&R, ray, Port Royal - END +bool CvUnitAI::AI_sailToOffMapTradeDestination(bool bMove) +{ + // Determine the correct off-map destination from the group's trade routes + CvSelectionGroup* pGroup = getGroup(); + if (pGroup != NULL) + { + CvPlayerAI& kOwner = GET_PLAYER(getOwnerINLINE()); + const std::set& tradeRoutes = pGroup->getTradeRoutes(); + for (std::set::const_iterator it = tradeRoutes.begin(); it != tradeRoutes.end(); ++it) + { + CvTradeRoute* pRoute = kOwner.getTradeRoute(*it); + if (pRoute != NULL) + { + int iDestID = pRoute->getDestinationCity().iID; + if (iDestID == CvTradeRoute::AFRICA_CITY_ID) + { + return AI_sailToAfrica(bMove); + } + if (iDestID == CvTradeRoute::PORT_ROYAL_CITY_ID) + { + return AI_sailToPortRoyal(bMove); + } + } + } + } + // Default to Europe if no Africa/Port Royal route found + return AI_sailToEurope(bMove); +} + /// Attempts to sail (off the map) to the specified port. /// A helper struct that contains port travel constants required by the AI /// If true, allows the unit to attempt to generate a path to the closest Euro plot, If false only check the current plot. diff --git a/Project Files/DLLSources/CvUnitAI.h b/Project Files/DLLSources/CvUnitAI.h index 1b7995e52..e93f728d5 100644 --- a/Project Files/DLLSources/CvUnitAI.h +++ b/Project Files/DLLSources/CvUnitAI.h @@ -187,6 +187,7 @@ class CvUnitAI : public CvUnit bool AI_sailToEurope(bool bMove = true); bool AI_sailToAfrica(bool bMove = true); /*** TRIANGLETRADE 10/28/08 by DPII ***/ bool AI_sailToPortRoyal(bool bMove = true); // R&R, ray, Port Royal + bool AI_sailToOffMapTradeDestination(bool bMove = true); CvPlot* findNearbyOceanPlot(const CvPlot& kPlot) const; // TAC - AI Improved Naval AI - koma13 From 4e301b5749047f268376edbfce67c558fb0c27a0 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 23:21:31 -0700 Subject: [PATCH 07/13] Add determinism tests (Phase 4) and Python validation tests (Phase 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4: CvRandom determinism tests — seed reproducibility, range bounds, peek non-advancement, float output, and state round-trips (576 checks). Phase 5: Python-side test suite (WTPTests.py) validating the DLL-Python bridge and XML data consistency — profession yields, building classes, unit professions, father categories, terrain yields, civilization info. Runnable from debug console: import WTPTests; WTPTests.runAllTests() Also fixes WTPTests.log double-nesting (gDLL->logMsg already writes to the Logs/ directory). Co-Authored-By: Claude Opus 4.6 --- Assets/Python/_DebugTools/WTPTests.py | 172 ++++++++++++++++++ Project Files/DLLSources/TestDeterminism.cpp | 157 ++++++++++++++++ Project Files/DLLSources/WTPTestFramework.cpp | 4 +- Project Files/RaR.vcxproj | 1 + 4 files changed, 333 insertions(+), 1 deletion(-) create mode 100644 Assets/Python/_DebugTools/WTPTests.py create mode 100644 Project Files/DLLSources/TestDeterminism.cpp diff --git a/Assets/Python/_DebugTools/WTPTests.py b/Assets/Python/_DebugTools/WTPTests.py new file mode 100644 index 000000000..f2bf673e3 --- /dev/null +++ b/Assets/Python/_DebugTools/WTPTests.py @@ -0,0 +1,172 @@ +## WTP Test Suite - Python Tests (Tier 3) +## Callable from the in-game debug console or via CvEventManager hook. +## Tests exercise the Python-DLL bridge and validate XML data integrity. +## +## Usage: from the debug console or Python event hook, call: +## import WTPTests +## WTPTests.runAllTests() + +from CvPythonExtensions import * +import CvUtil + +gc = CyGlobalContext() + +# ============================================================================ +# Test Infrastructure +# ============================================================================ + +class WTPTestRunner: + def __init__(self): + self.tests_run = 0 + self.tests_passed = 0 + self.tests_failed = 0 + self.failures = [] + + def reset(self): + self.tests_run = 0 + self.tests_passed = 0 + self.tests_failed = 0 + self.failures = [] + + def runTest(self, name, testFunc): + self.tests_run = self.tests_run + 1 + errors = testFunc() + if len(errors) == 0: + self.tests_passed = self.tests_passed + 1 + else: + self.tests_failed = self.tests_failed + 1 + for e in errors: + self.failures.append("%s: %s" % (name, e)) + + def report(self): + if self.tests_failed == 0: + msg = "[PASS] WTPTests.py: %d/%d tests passed" % (self.tests_passed, self.tests_run) + CvUtil.pyPrint(msg) + CyInterface().addImmediateMessage(msg, "") + else: + msg = "[FAIL] WTPTests.py: %d/%d tests failed" % (self.tests_failed, self.tests_run) + CvUtil.pyPrint(msg) + CyInterface().addImmediateMessage(msg, "") + for f in self.failures: + CvUtil.pyPrint(" " + f) + CyInterface().addImmediateMessage(f, "") + +g_runner = WTPTestRunner() + +# ============================================================================ +# Test: Profession Yields Valid +# ============================================================================ + +def testProfessionYieldsValid(): + errors = [] + numYields = gc.getNumYieldInfos() + for i in range(gc.getNumProfessionInfos()): + info = gc.getProfessionInfo(i) + for j in range(info.getNumYieldsProduced()): + eYield = info.getYieldsProduced(j) + if eYield != -1 and (eYield < 0 or eYield >= numYields): + errors.append("Profession %d has invalid produced yield %d" % (i, eYield)) + for j in range(info.getNumYieldsConsumed()): + eYield = info.getYieldsConsumed(j) + if eYield != -1 and (eYield < 0 or eYield >= numYields): + errors.append("Profession %d has invalid consumed yield %d" % (i, eYield)) + return errors + +# ============================================================================ +# Test: Building Classes Valid +# ============================================================================ + +def testBuildingClassesValid(): + errors = [] + numClasses = gc.getNumBuildingClassInfos() + for i in range(gc.getNumBuildingInfos()): + info = gc.getBuildingInfo(i) + iClass = info.getBuildingClassType() + if iClass < 0 or iClass >= numClasses: + errors.append("Building %d has invalid class %d" % (i, iClass)) + return errors + +# ============================================================================ +# Test: Unit Default Professions Valid +# ============================================================================ + +def testUnitDefaultProfessions(): + errors = [] + numProfessions = gc.getNumProfessionInfos() + for i in range(gc.getNumUnitInfos()): + info = gc.getUnitInfo(i) + eProfession = info.getDefaultProfession() + if eProfession != -1 and (eProfession < 0 or eProfession >= numProfessions): + errors.append("Unit %d has invalid default profession %d" % (i, eProfession)) + return errors + +# ============================================================================ +# Test: Father Categories Valid +# ============================================================================ + +def testFatherCategoriesValid(): + errors = [] + numCategories = gc.getNumFatherCategoryInfos() + for i in range(gc.getNumFatherInfos()): + info = gc.getFatherInfo(i) + eCategory = info.getFatherCategory() + if eCategory < 0 or eCategory >= numCategories: + errors.append("Father %d has invalid category %d" % (i, eCategory)) + return errors + +# ============================================================================ +# Test: Yield Infos Cargo Flag Consistency +# ============================================================================ + +def testYieldCargoConsistency(): + errors = [] + for i in range(gc.getNumYieldInfos()): + info = gc.getYieldInfo(i) + # Every yield should have a valid type string + typeName = info.getType() + if typeName is None or len(typeName) == 0: + errors.append("Yield %d has empty type string" % i) + return errors + +# ============================================================================ +# Test: Terrain Yield Access +# ============================================================================ + +def testTerrainYieldAccess(): + errors = [] + numYields = gc.getNumYieldInfos() + for i in range(gc.getNumTerrainInfos()): + info = gc.getTerrainInfo(i) + for j in range(numYields): + # Should not crash + val = info.getYield(j) + return errors + +# ============================================================================ +# Test: Civilization Info Consistency +# ============================================================================ + +def testCivilizationInfoConsistency(): + errors = [] + for i in range(gc.getNumCivilizationInfos()): + info = gc.getCivilizationInfo(i) + typeName = info.getType() + if typeName is None or len(typeName) == 0: + errors.append("Civilization %d has empty type string" % i) + return errors + +# ============================================================================ +# Test Runner +# ============================================================================ + +def runAllTests(): + g_runner.reset() + g_runner.runTest("ProfessionYieldsValid", testProfessionYieldsValid) + g_runner.runTest("BuildingClassesValid", testBuildingClassesValid) + g_runner.runTest("UnitDefaultProfessions", testUnitDefaultProfessions) + g_runner.runTest("FatherCategoriesValid", testFatherCategoriesValid) + g_runner.runTest("YieldCargoConsistency", testYieldCargoConsistency) + g_runner.runTest("TerrainYieldAccess", testTerrainYieldAccess) + g_runner.runTest("CivilizationInfoConsistency", testCivilizationInfoConsistency) + g_runner.report() + return g_runner.failures diff --git a/Project Files/DLLSources/TestDeterminism.cpp b/Project Files/DLLSources/TestDeterminism.cpp new file mode 100644 index 000000000..e1d820321 --- /dev/null +++ b/Project Files/DLLSources/TestDeterminism.cpp @@ -0,0 +1,157 @@ + +#include "CvGameCoreDLL.h" +#include "WTPTestFramework.h" + +#ifdef FASSERT_ENABLE + +// ============================================================================ +// CvRandom Determinism Tests +// ============================================================================ + +WTP_TEST(Determinism, SameSeedSameSequence) +{ + CvRandom rng1; + CvRandom rng2; + rng1.init(42); + rng2.init(42); + + for (int i = 0; i < 100; ++i) + { + unsigned short val1 = rng1.get(1000, NULL); + unsigned short val2 = rng2.get(1000, NULL); + WTP_ASSERT_EQ(val1, val2); + } +} + +WTP_TEST(Determinism, DifferentSeedsDifferentSequence) +{ + CvRandom rng1; + CvRandom rng2; + rng1.init(42); + rng2.init(99); + + // At least one of the first 10 values should differ + bool bDifferent = false; + for (int i = 0; i < 10; ++i) + { + if (rng1.get(1000, NULL) != rng2.get(1000, NULL)) + { + bDifferent = true; + break; + } + } + WTP_ASSERT(bDifferent); +} + +WTP_TEST(Determinism, ReseedRestoresSequence) +{ + CvRandom rng; + rng.init(12345); + + // Generate some values + unsigned short first = rng.get(1000, NULL); + unsigned short second = rng.get(1000, NULL); + + // Reseed to same value + rng.reseed(12345); + + // Should produce same sequence + WTP_ASSERT_EQ(first, rng.get(1000, NULL)); + WTP_ASSERT_EQ(second, rng.get(1000, NULL)); +} + +WTP_TEST(Determinism, PeekDoesNotAdvance) +{ + CvRandom rng; + rng.init(42); + + unsigned long seedBefore = rng.getSeed(); + unsigned long peeked = rng.peek(); + unsigned long seedAfter = rng.getSeed(); + + // Peek should not change state + WTP_ASSERT_EQ(seedBefore, seedAfter); + + // Peek should predict the next state + rng.get(1000, NULL); + // After advancing, seed should match what peek predicted + // peek() returns the raw next seed, get() also advances + // So we verify peek returned something different from current + WTP_ASSERT(peeked != seedBefore); +} + +WTP_TEST(Determinism, GetRangeValid) +{ + CvRandom rng; + rng.init(777); + + for (int i = 0; i < 200; ++i) + { + unsigned short val = rng.get(100, NULL); + WTP_ASSERT(val < 100); + } +} + +WTP_TEST(Determinism, GetRange1AlwaysZero) +{ + CvRandom rng; + rng.init(42); + + for (int i = 0; i < 50; ++i) + { + WTP_ASSERT_EQ(0, rng.get(1, NULL)); + } +} + +WTP_TEST(Determinism, FloatInRange) +{ + CvRandom rng; + rng.init(42); + + for (int i = 0; i < 100; ++i) + { + float f = rng.getFloat(); + WTP_ASSERT(f >= 0.0f); + WTP_ASSERT(f <= 1.0f); + } +} + +WTP_TEST(Determinism, SeedSurvivesRoundTrip) +{ + CvRandom rng; + rng.init(54321); + // Advance state a few times + rng.get(100, NULL); + rng.get(100, NULL); + unsigned long savedSeed = rng.getSeed(); + + CvRandom rng2; + rng2.reseed(savedSeed); + WTP_ASSERT_EQ(savedSeed, rng2.getSeed()); + + // Both should produce identical sequences from here + for (int i = 0; i < 20; ++i) + { + WTP_ASSERT_EQ(rng.get(500, NULL), rng2.get(500, NULL)); + } +} + +// ============================================================================ +// Test Runner +// ============================================================================ + +void RunDeterminismTests() +{ + WTP_RUN_SUITE_BEGIN(); + WTP_RUN_TEST(Determinism, SameSeedSameSequence); + WTP_RUN_TEST(Determinism, DifferentSeedsDifferentSequence); + WTP_RUN_TEST(Determinism, ReseedRestoresSequence); + WTP_RUN_TEST(Determinism, PeekDoesNotAdvance); + WTP_RUN_TEST(Determinism, GetRangeValid); + WTP_RUN_TEST(Determinism, GetRange1AlwaysZero); + WTP_RUN_TEST(Determinism, FloatInRange); + WTP_RUN_TEST(Determinism, SeedSurvivesRoundTrip); + WTP_RUN_SUITE_END("Determinism"); +} + +#endif diff --git a/Project Files/DLLSources/WTPTestFramework.cpp b/Project Files/DLLSources/WTPTestFramework.cpp index 6b2b87e6c..0961478a5 100644 --- a/Project Files/DLLSources/WTPTestFramework.cpp +++ b/Project Files/DLLSources/WTPTestFramework.cpp @@ -6,7 +6,7 @@ WTPTestResult g_wtpTestResult; -static const char* LOG_FILE = "Logs\\WTPTests.log"; +static const char* LOG_FILE = "WTPTests.log"; void WTPTestReport(const char* suiteName, const WTPTestResult& result) { @@ -36,6 +36,7 @@ void WTPTestReport(const char* suiteName, const WTPTestResult& result) void RunDataStructureTests(); void RunXMLIntegrityTests(); void RunSavegameTests(); +void RunDeterminismTests(); void RunAllWTPTests() { @@ -44,6 +45,7 @@ void RunAllWTPTests() RunDataStructureTests(); RunXMLIntegrityTests(); RunSavegameTests(); + RunDeterminismTests(); gDLL->logMsg(LOG_FILE, "=== WTP Test Suite Complete ==="); } diff --git a/Project Files/RaR.vcxproj b/Project Files/RaR.vcxproj index 556758631..65077535d 100644 --- a/Project Files/RaR.vcxproj +++ b/Project Files/RaR.vcxproj @@ -123,6 +123,7 @@ + From 26b9839eba0111fa8b4bed1d72b381c7e9ed028f Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 23:21:33 -0700 Subject: [PATCH 08/13] Add .claude to .gitignore Co-Authored-By: Claude Opus 4.6 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7b503a643..d62ce6c42 100644 --- a/.gitignore +++ b/.gitignore @@ -27,5 +27,6 @@ Thumbs.db /Project Files/Boost-1.32.0 /Project Files/Python24 .idea +.claude /Assets/EditorCache.xml /Assets/EditorSettings.xml From 360cad2108cc769baea5798c062c786ba9479430 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 13 Mar 2026 23:49:01 -0700 Subject: [PATCH 09/13] Fix yield modifier assert for YIELD_LAW in city tooltip setCityYieldModifierString was missing the YIELD_LAW exclusion when applying the rebel yield modifier, causing a mismatch with getBaseYieldRateModifier which correctly excludes it. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/CvGameTextMgr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project Files/DLLSources/CvGameTextMgr.cpp b/Project Files/DLLSources/CvGameTextMgr.cpp index 7d9f5e2c3..7822eb860 100644 --- a/Project Files/DLLSources/CvGameTextMgr.cpp +++ b/Project Files/DLLSources/CvGameTextMgr.cpp @@ -9344,7 +9344,7 @@ int CvGameTextMgr::setCityYieldModifierString(CvWStringBuffer& szBuffer, YieldTy // WTP, ray, trying to fix Rebel Rate Modifier on Happiness for Balancing - START // just if condition added - if (eYieldType != YIELD_HAPPINESS && eYieldType != YIELD_UNHAPPINESS && eYieldType != YIELD_CRIME) + if (eYieldType != YIELD_HAPPINESS && eYieldType != YIELD_UNHAPPINESS && eYieldType != YIELD_LAW && eYieldType != YIELD_CRIME) { int iRebelMod = kCity.getRebelPercent() * GC.getMAX_REBEL_YIELD_MODIFIER() / 100; if (0 != iRebelMod) From b13b9eed99a4d9f9bf323c4142c83db609d0665b Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 27 Mar 2026 14:23:47 -0700 Subject: [PATCH 10/13] Fix event random numbers never being stored in setRandomNumbers() EventTriggeredData::setRandomNumbers() creates a RandomContainer for each event in the trigger, generates a random number via getSorenRandNum, but never actually stores the container in m_RandomNumbers. The push_back call was missing, so m_RandomNumbers remained empty after the function completed. This caused getRandomNumber() and getRandomNumberForIndex() to always return 0 (the fallback default), which had two consequences: 1. Any Python PythonCanDo callback using getRandomNumberForIndex() for probability checks would always see 0. For example, canTriggerVolcanoDormant1() checks "getRandomNumberForIndex(0) < 250" which was always true (0 < 250), making the volcano dormant event fire 100% of the time instead of the intended ~25%. 2. The DLL-side TriggerChance fastpath in CvPlayer::canDoEvent() (line 14762) uses getRandomNumber(eEvent) to check event probability thresholds. With the vector always empty, this also always returned 0, bypassing intended probability gates for events validated through that code path. The fix adds the missing m_RandomNumbers.push_back(container) so that generated random numbers are actually persisted in the vector and available to both Python and C++ callers. Related: #1205 (CvRandomInterfaceEvent tuple index out of range) The primary cause of #1205 was an argsList index mismatch fixed in 38b75188, but this bug compounded the issue by making the random number check in canTriggerVolcanoDormant1 a no-op. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/Events/EventTrigger.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Project Files/DLLSources/Events/EventTrigger.cpp b/Project Files/DLLSources/Events/EventTrigger.cpp index ed1561fee..dab540bbb 100644 --- a/Project Files/DLLSources/Events/EventTrigger.cpp +++ b/Project Files/DLLSources/Events/EventTrigger.cpp @@ -71,6 +71,7 @@ void EventTriggeredData::setRandomNumbers() RandomContainer container; container.event = eEvent; container.number = GC.getGameINLINE().getSorenRandNum(1000, "Event random number"); + m_RandomNumbers.push_back(container); } } } From 96032abed833a8c5b3383d754fc04f7381f2d93f Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Fri, 27 Mar 2026 14:25:29 -0700 Subject: [PATCH 11/13] Fix copy-paste bug using m_iPlotX for both X and Y in initTriggeredData In CvPlayer::initTriggeredData(), after a PythonCanDo callback returns successfully, the code reconstructs a Coordinates object from the trigger data (since Python may have modified it). However, line 14416 passed m_iPlotX as both the X and Y arguments: Coordinates coord(pTriggerData->m_iPlotX, pTriggerData->m_iPlotX); This meant pPlot was resolved to the wrong tile whenever the Python callback modified the trigger's plot coordinates and X != Y. The resolved plot would be at (X, X) instead of (X, Y), causing the event to target the wrong map location for any subsequent logic that uses pPlot (text generation, world news, event application). Events where PythonCanDo does not modify coordinates would be unaffected since pPlot was already correctly set before the callback. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/CvPlayer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project Files/DLLSources/CvPlayer.cpp b/Project Files/DLLSources/CvPlayer.cpp index 577b4666e..b4bbf886c 100644 --- a/Project Files/DLLSources/CvPlayer.cpp +++ b/Project Files/DLLSources/CvPlayer.cpp @@ -14422,7 +14422,7 @@ EventTriggeredData* CvPlayer::initTriggeredData(EventTriggerTypes eEventTrigger, return NULL; } - Coordinates coord (pTriggerData->m_iPlotX, pTriggerData->m_iPlotX); + Coordinates coord (pTriggerData->m_iPlotX, pTriggerData->m_iPlotY); // python may change pTriggerData pCity = getCity(pTriggerData->m_iCityId); From ad5aeab14e226d196492cf0f425ea96b79ba30a6 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Sun, 29 Mar 2026 14:17:51 -0700 Subject: [PATCH 12/13] Prevent native-owned ships from sailing to Europe/Africa/Port Royal Natives have no Europe dock, so if they acquire an ocean-going ship (e.g. via gifting) and it sails to Europe, the game crashes accessing non-existent dock units. Add isNative() checks to canCrossOcean, canSailToAfrica, and canSailToPortRoyal to block the trip entirely. Fixes #1039 Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/CvUnit.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Project Files/DLLSources/CvUnit.cpp b/Project Files/DLLSources/CvUnit.cpp index b0ffd73e7..fa7ffeef8 100644 --- a/Project Files/DLLSources/CvUnit.cpp +++ b/Project Files/DLLSources/CvUnit.cpp @@ -5024,6 +5024,12 @@ bool CvUnit::canAutoCrossOcean(const CvPlot* pPlot) const bool CvUnit::canCrossOcean(const CvPlot* pPlot, UnitTravelStates eNewState) const { + // Natives have no Europe/Africa/Port Royal — prevent crash when they acquire ocean ships + if (isNative()) + { + return false; + } + if (getTransportUnit() != NULL) { return false; @@ -5105,6 +5111,11 @@ void CvUnit::crossOcean(UnitTravelStates eNewState) /*** TRIANGLETRADE 10/28/08 by DPII ***/ bool CvUnit::canSailToAfrica(const CvPlot* pPlot, UnitTravelStates eNewState) const { + if (isNative()) + { + return false; + } + if (getTransportUnit() != NULL) { return false; @@ -5191,6 +5202,11 @@ void CvUnit::sailToAfrica(UnitTravelStates eNewState) // R&R, ray, Port Royal bool CvUnit::canSailToPortRoyal(const CvPlot* pPlot, UnitTravelStates eNewState) const { + if (isNative()) + { + return false; + } + // only Ships with hidden nationality can sail to Port Royal // WTP, ray Slave Ship // we allow Slave Ships to sail to Port Royal as well From b6c886655cd6c9816223a5896aba0c5bce217e52 Mon Sep 17 00:00:00 2001 From: Alessandro Catorcini Date: Mon, 30 Mar 2026 12:38:09 -0700 Subject: [PATCH 13/13] Use parent/king check instead of isNative() for ocean travel guard Replace isNative() check with a positive identity check: only players with a parent (colonial nations) or kings (isEurope) can sail to Europe/Africa/Port Royal. This also covers wild animals and other non-standard player types. Co-Authored-By: Claude Opus 4.6 --- Project Files/DLLSources/CvUnit.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Project Files/DLLSources/CvUnit.cpp b/Project Files/DLLSources/CvUnit.cpp index fa7ffeef8..5022b0691 100644 --- a/Project Files/DLLSources/CvUnit.cpp +++ b/Project Files/DLLSources/CvUnit.cpp @@ -5024,8 +5024,9 @@ bool CvUnit::canAutoCrossOcean(const CvPlot* pPlot) const bool CvUnit::canCrossOcean(const CvPlot* pPlot, UnitTravelStates eNewState) const { - // Natives have no Europe/Africa/Port Royal — prevent crash when they acquire ocean ships - if (isNative()) + // Only colonial nations (have a parent/king) and kings can sail to Europe/Africa/Port Royal + const CvPlayer& kOwner = GET_PLAYER(getOwnerINLINE()); + if (kOwner.getParent() == NO_PLAYER && !kOwner.isEurope()) { return false; } @@ -5111,7 +5112,8 @@ void CvUnit::crossOcean(UnitTravelStates eNewState) /*** TRIANGLETRADE 10/28/08 by DPII ***/ bool CvUnit::canSailToAfrica(const CvPlot* pPlot, UnitTravelStates eNewState) const { - if (isNative()) + const CvPlayer& kOwner = GET_PLAYER(getOwnerINLINE()); + if (kOwner.getParent() == NO_PLAYER && !kOwner.isEurope()) { return false; } @@ -5202,7 +5204,8 @@ void CvUnit::sailToAfrica(UnitTravelStates eNewState) // R&R, ray, Port Royal bool CvUnit::canSailToPortRoyal(const CvPlot* pPlot, UnitTravelStates eNewState) const { - if (isNative()) + const CvPlayer& kOwner = GET_PLAYER(getOwnerINLINE()); + if (kOwner.getParent() == NO_PLAYER && !kOwner.isEurope()) { return false; }