Skip to content

Village.php full REFACTOR and IMPROVEMENTS #326

Description

@Shadowss
  1. Production clamp in processProduction()
    Right now: $nwood = min(prod * time, $maxstore)
    Correct: min(prod * time, $maxstore - $current)
    Otherwise a long offline period gives you a full warehouse instantly. Same for clay/iron/crop. And clamp crop at 0, otherwise starvation goes negative forever.
  2. Double DB write
    LoadTown() does an overflow fix + updateResource(), then processProduction() does modifyResource() + updateVillage(). That's 2-3 UPDATEs per hit. Do the overflow clamp in-memory only, write once.
  3. $this->wid = $this->infoarray['wref']
    This overwrites your village id mid-load. Kill that line.
  4. Automation lock race
    file_exists() + file_put_contents() is a classic TOCTOU. Switch to fopen + flock(), 3 lines.
    All four are patchable in 30 minutes, drop-in, no API change.
    In-file refactor timeline, keeping Village.php as one file
    Day 1 – Correctness patch
    The 4 fixes above. Ship it. Game logic stays 1:1.
    Week 1 – De-duplication, same class
  5. Merge the 4 copy-paste getWoodProd / getClayProd / getIronProd / getCropProd into one getProdFor(int $type) helper. Pass in the $bidX table, the bonus building id, and the session bonus flag. Cuts ∼120 lines, and you only loop fields 1..38 once instead of 4 times.
  6. Add types. public int $wid, private array $production, function getProd(string $type): int. PHP 8.2, no breakage.
  7. Stop repeating global $bid1...$bid9, $session in every method. Pull them once in __construct() into private properties. Still globals under the hood, but contained.
    Week 2 – Kill constructor side effects
  8. __construct() only calls LoadTown(). Nothing else.
  9. Move calculateProduction() + processProduction() into a public tick() method, called explicitly from dorf1.php / dorf2.php.
  10. Pull ActionControl() with its header("Location: dorf1.php"); exit; out of the model entirely, put it at the top of build.php before new Village.
  11. Delete static $loadedWid / $cacheCleared. With an explicit tick you don't need request-level guards anymore. You can instantiate multiple villages, admin switch works again.
    After this, new Village is safe to call. No DB writes, no redirects.
    Week 3 – I/O consolidation, still in LoadTown()
    Don't create a separate Repository. Just make your Database class expose one getVillageFull($wid) with JOINs for vdata, fdata, oasis, units. In LoadTown() you map the result into the same $this->resarray, $this->unitarray, etc. Public API of Village stays identical, the rest of TravianZ notices nothing.
    Result: ∼2 queries per view instead of 12-14.
    End state: still one Village.php, ∼350 lines instead of 500+, no globals scattered in methods, production bug fixed, no side effects in the constructor, and the query count cut by 80%.

Metadata

Metadata

Labels

BugSomething isn't workingEnhancementNew feature or requestHelp wantedExtra attention is needed

Projects

Status
No status

Relationships

None yet

Development

No branches or pull requests

Issue actions