Fix correctness bugs and UX gaps in the housing stack#106
Merged
Conversation
009b629 to
fd9f809
Compare
…g stack Multi-angle review of the home-ownership/investments/homeless-and-renting stack (PRs #104, #105) turned up several real issues, all fixed here: Correctness: - Player.__init__ hardcoded energy=100, but a fresh player's homeTier=0 (Homeless) caps at 60 - new players started 40 energy over their own cap. Now derived from housing.HOUSING_TIERS[0]["maxEnergy"]. - moveHome() changed player.homeTier but never reclamped player.energy to the new (possibly lower) cap, so downgrading or being evicted left the player over-cap until their next sleep. Eviction is now routed through moveHome() (instead of duplicating the tier assignment inline), so it gets this reconciliation automatically. - investments.typeInfo()/housing.tierInfo() silently wrapped around via Python's negative-index behavior on an out-of-range id instead of failing loudly; both now raise ValueError, and schemas/player.json gained the matching maximum bounds on homeTier/rentalProperties. UX (eviction was completely silent - the player could lose their room and never be told): - TimeService.increaseDay()/increaseTime() now return {"evicted": bool}. Every place a day can roll over (Home.sleep, Tavern.getDrunk, a multi-hour Docks.fish trip, and the main game loop as a catch-all for everything else) surfaces housing.EVICTION_MESSAGE when it happens. - "Move to Rented Room (free)" was misleading - it now discloses the recurring $10/day rent up front, before the player commits. "+$X" cash-back downgrade label is now "get $X back". - Energy is now shown as "current/cap" (not just current) in all three front-ends (console, pygame, web), so the housing energy-cap benefit is visible without opening Manage Home. Cleanup: - netCostToMove() simplified (dict.get defaults already encode "only owned tiers have a cost/resaleValue", the status-gated ternaries were redundant). - home.py's repeated adjacent-tier-move computation (built separately in _housingStatus() and manageHome()) is now computed once via _availableMoves(). - manageInvestments() combined its two PROPERTY_TYPES loops (buy options, then sell options) into one pass, backed by a new investments.ownedCounts() that computes the whole portfolio's counts in one pass instead of an O(types) list.count() scan per type per render. Deliberately not changed (flagged in review, judged not worth the churn): menu-loop duplication across manageHome/manageInvestments/manageBusiness, investments' 1-based vs housing's 0-based tier indexing, and displayStats()'s hand-rolled optional-block style vs achievements.py's data-driven MILESTONES pattern. Stacked on feature/homeless-and-renting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
889462a to
2ed044d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #105
This targets
feature/homeless-and-renting(#105), notmain— review it last, after #104 and #105.Summary
A full multi-angle review (line-by-line, removed-behavior, cross-file, reuse, simplification, efficiency, altitude, conventions, and UX) of the whole housing/investments/homeless-and-renting stack turned up real issues. All fixed here.
Correctness
Player.__init__hardcodedenergy=100, but a fresh player'shomeTier=0(Homeless) caps at 60 — new players started 40 energy over their own cap. Now derived fromhousing.HOUSING_TIERS[0]["maxEnergy"].moveHome()changedplayer.homeTierbut never reclampedplayer.energyto the new (possibly lower) cap, so downgrading or being evicted left the player over-cap until their next sleep. Eviction now routes throughmoveHome()instead of duplicating the tier assignment inline, so it gets this reconciliation automatically.investments.typeInfo()/housing.tierInfo()silently wrapped around via Python's negative-index behavior on an out-of-range id instead of failing loudly; both now raiseValueError, andschemas/player.jsongained matchingmaximumbounds.UX — eviction was completely silent
TimeService.increaseDay()/increaseTime()now return{"evicted": bool}. Every place a day can roll over (Home.sleep,Tavern.getDrunk, a multi-hourDocks.fishtrip, and the main game loop as a catch-all) surfaceshousing.EVICTION_MESSAGEwhen it happens.Cleanup
netCostToMove()simplified (the status-gated ternaries were redundant withdict.getdefaults).home.py's repeated adjacent-tier-move computation (built separately in_housingStatus()andmanageHome()) is now computed once.manageInvestments()combined twoPROPERTY_TYPESloops into one pass, backed by a newinvestments.ownedCounts().Deliberately not changed (flagged, not worth the churn): menu-loop duplication across
manageHome/manageInvestments/manageBusiness, investments' 1-based vs housing's 0-based indexing,displayStats()'s hand-rolled optional blocks vsachievements.py's data-driven pattern.Test plan
python3 -m pytest --verbose -vv --cov=src --cov-report=term-missing— 325 passed (up from 309 onfeature/homeless-and-renting).🤖 Generated with Claude Code