The Brewery (building 35, Teuton-only) grants a +1% attack bonus per level
to the whole empire, but only while a Mead-Festival is running (72h), and the
building is capital-only. The attack-bonus block in calculateBattle() ignored
both rules: it read the Brewery level from $AttackerWref (the *launching*
village, which usually has no Brewery) and applied the bonus purely on the
building level, with no festival check.
As a result the bonus never reacted to the festival being started or expired:
attacks with and without an active festival produced identical losses, so the
army appeared unaffected by the festival (issue #294). Depending on whether the
attack was launched from the capital, the bonus was either permanently on or
permanently off, but never gated by the festival.
Read the Brewery level from the attacker's capital and gate the bonus on the
festival being active, mirroring the catapult-randomization gate in Units.php
and the chief-penalty gate in Automation.php. The simulator passes
AttackerID = 0, so it keeps a 0 bonus exactly as before.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
procDistanceTime() multiplied the whole travel distance by the Tournament
Square speed factor as soon as the distance reached TS_THRESHOLD. That
made the trip time jump down at the threshold, so a target just past it
arrived dramatically sooner than a nearer one (e.g. a village 41 tiles
away raided faster than one 18 tiles away).
In T3.6 the Tournament Square only speeds up the part of the journey
beyond the threshold: the first TS_THRESHOLD tiles are walked at base
speed and the remainder at the boosted speed. Split the computation
accordingly so travel time stays monotonic with distance while still
rewarding a high-level square.
This is a long-standing bug, unrelated to the Generator refactor (which
only reformatted the same whole-distance multiplication). The same fix is
applied to the duplicate procDistanceTime() in Admin/database.php used by
the admin troop-return helper.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to #309. That fix only skipped follow-up waves whose target
was razed within the same resolution batch (an in-memory set). But waves
timed a second apart land in separate ticks: the first razes the village,
the next tick re-resolves the follow-up wave against a village that no
longer exists. getMInfo() (wdata LEFT JOIN vdata) then returns NULL vdata
columns, so the return trip is computed from a NULL wref (NULL coords) ->
a bogus arrival time that strands the troops forever (report against
"[?]"); DelVillage() does not reliably bounce every in-flight wave
either, leaving the attack at proc=0 and re-fetched every tick.
Detect the razed target from DB truth: after resolveVillageTarget(), if
the village is gone ($to['wref'] is NULL) or it was razed earlier in this
same batch ($razedTargets, still needed because getMInfo() is cached and
a same-batch wave would otherwise see the stale "alive" row), bounce the
whole army straight home and mark the movement processed instead of
fighting a phantom. setMovementProc() returns true only when it flips
proc 0->1, so we never duplicate a return DelVillage() already created.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
When several catapult waves land in the same resolution tick,
sendunitsComplete() fetches them all into one in-memory batch. The first
wave can raze the target village: handleVillageDestruction() -> DelVillage()
already bounces every still-in-flight attack straight home (symmetric return
time) and deletes the vdata row, leaving the wdata tile as an empty valley.
The loop then kept resolving the remaining waves from the in-memory batch
against the now-deleted village. getMInfo() LEFT-JOINs wdata onto the missing
vdata, so every vdata column (including wref) comes back NULL. The return
trip in finalizeReturnOrDeath() is then computed from a NULL wref (NULL
coordinates), producing a bogus arrival time that strands the troops, and a
duplicate of the bounce DelVillage() had already issued. The attacker sees
the waves "coming to nowhere and never coming back" (issue #298).
Track the tiles razed during the current batch and skip any follow-up wave
that targets one of them: those troops were already bounced home by
DelVillage(). No change to the battle maths; for a normal (non-razed) target
the set stays empty, so behaviour is preserved.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
generateBase()'s four sector predicates overlapped on the axes: a tile
with x = 0 or y = 0 satisfied two of them (e.g. both "x <= 0" and
"x >= 0" matched x = 0). createWWVillages() and addArtifactVillages()
generate a whole batch in one go and only flip occupied = 0 -> 1 at the
very end (setFieldTaken()), so two sectors could each select the SAME
axis tile and assign its wref to two different villages. The duplicate
addVillage() insert then violated the vdata PRIMARY KEY(wref); under the
PHP 8.1+ mysqli exception mode this threw and aborted generateVillages()
before setFieldTaken()/addResourceFields()/addUnits() ran. The result:
the villages existed in vdata (and showed up under the Natars profile)
but their map tile was never marked occupied and had no resource fields
or troops, so the map still rendered an "abandoned valley" (issue #301).
Make the four sectors mutually exclusive by using a strict bound on one
side of each axis, so every tile belongs to exactly one sector and no
wref can be handed out twice within a batch.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Issue #299: posting to an admin Mod (eg editBuildings.php) could show an
essentially blank page. The admin panel and the game share the same PHP
session, so a game logout (session_destroy) — or a mobile browser dropping the
session cookie / serving a cached form with a stale token — wipes the admin
session. The Mod then stopped on a bare die('<h1>Access Denied</h1>') (or the
403 die() in csrf_verify()), which renders as a blank/broken page outside the
panel.
Add a shared admin_deny() helper in GameEngine/Admin/csrf.php that renders a
clean, self-contained, styled error page (with a "Return to Admin Panel" link)
and a no-store header, then exits. Wire it into csrf_verify() and replace every
bare "Access Denied" die() across the 42 admin Mods. Each Mod now loads
csrf.php at the top so admin_deny() is available before its first access check.
This is the presentation fix Shadow asked for ("we must receive an error not
blank page"). The deeper root cause (admin and game sharing one PHP session) is
left for a follow-up: giving the admin panel its own session cookie name.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Bug fix: $oasisowned was fetched but never applied — this is the
"time to overflow" timer's own, independent recomputation of the
production rate (used only for display here), and it silently
excluded oasis bonuses entirely. Village::getCropProd()/getWoodProd()
etc. (which actually grow the stored resource in the DB on every
page load via processProduction()) do add a flat 25% per matching
oasis (Village::sortOasis()), so the real stockpile was growing
faster than this timer's denominator assumed — understating the
rate, and therefore overstating the time remaining. Mirrored here
with the same counting + application order (oasis bonus on the raw
field total, then the building bonus below on top of that), so this
rate matches the one actually used to fill the storage.
// Bug fix: RemoveXSS() calls htmlspecialchars() (&,<,>,",' -> entities).
// Every display site for these values ALREADY escapes correctly on output
// (links.tpl's safeHTML(), and preference.tpl's edit-row value=""), so
// encoding here too meant a saved "&" was stored as literal "&" text
// in the DB, then got escaped AGAIN on redisplay — surviving one level of
// browser entity-decoding as visible "&". Worse, it silently broke
// any saved link with a real query parameter after the first one (e.g.
// build.php?gid=16&t=99): the stored value no longer had a real "&"
// separator there, so "t" was never received as its own GET param.
// strip_tags() (for name) + mysqli_real_escape_string() (below, for SQL)
// are sufficient at save time; HTML-escaping belongs only at display time.
sendTroops() inlined ~65 lines deciding the catapult targets ctar1/ctar2: the
"Rivals great confusion" artefact lookup, the rally-point-level-driven list of
invalid target buildings, the troop/level eligibility rules and the Teuton
Brewery / artefact adjustments. Move that whole block into
resolveCatapultTargets(&$post, $data), which mutates $post['ctar1']/['ctar2'] by
reference exactly as before; sendTroops() now calls it before building the
attack. None of the block's locals were used afterwards. Behaviour-preserving.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Both branches of Hero() (single hero when !$all, full list when $all) computed
the same five derived stats (atk/di/dc/ob/db) and assembled a byte-identical
hero stat array from a getHero() entry plus its unit base data. Extract that
into buildHeroStats($hero, $herodata) and call it from both branches.
Behaviour-preserving.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Cases 1 to 4 of the procUnits() switch had a byte-identical body (send troops
when the rally-point form is submitted, otherwise load the unit form). Stack the
four case labels and keep a single shared body via switch fall-through.
Behaviour-preserving.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
upgradeSword() and upgradeArmour() were near-identical: the only differences
were the AB-tech key prefix ('b' vs 'a'), the building type whose level gates
the research (Smithy 12 vs Armoury 13) and the matching bid building data
($bid12 vs $bid13). Merge them into a single upgradeWeaponOrArmour($get, $type)
parameterised by the prefix, deriving the building type from it, and route both
procTechno() cases through it. Resolves the pre-existing //TODO. Behaviour-
preserving.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
novgorodschi catalin