Give the entity log cap a single source of truth - #44
Merged
Conversation
The per-entity log cap was declared twice: as a default parameter on LivingEntity.addLogEntry and as Config.entityLogMaxSize. Only one call site passed the configured value, so changing the setting from 50 would have made the two disagree and rebuilt the bounded deque on nearly every append -- an O(n) copy on the hot path the bounded log exists to keep O(1). The cap now belongs to the entity and is fixed at construction, and the config default reads the entity module's constant. Kreatures passes the configured cap to the world, the player creature, spawned creatures and children, so the setting reaches every entity in the game. Also removes src/stats/__init__ copy.py, a byte-identical editor artifact that nothing imports. Closes #40 Closes #43 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The deque's maxlen already carries the cap; a parallel attribute on LivingEntity and World was a second place for the same value to live, which is the duplication this change set exists to remove. Neither field was read anywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
Self-review rubricScored against the local pytest run (66 passed) and the diff, adversarially — assume FAIL until evidenced.
Findings
This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). |
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.
Summary
LivingEntity.addLogEntry(maxLogSize=50) and asConfig.entityLogMaxSize. Of every call site, onlyKreatures.createChildEntitypassed the configured value, so the setting was effectively inert and the two values would have diverged the moment either was changed.LivingEntity(name, maxLogSize=DEFAULT_LOG_MAX_SIZE)buildsself.logas an already-boundeddeque, andaddLogEntry(message)is a bare O(1) append with no cap argument. Theisinstance/maxlencheck that would have rebuilt the deque on nearly every call has been dropped, since divergence is no longer possible.DEFAULT_LOG_MAX_SIZEis defined once, insrc/entity/livingEntity.py, andConfig.entityLogMaxSizeis initialised from it. This adds one import fromconfigtoentity; it is what makes "one source of truth" literally true rather than two literals that happen to agree.World.__init__takesmaxLogSize), the player creature, creatures spawned bycreateEntity, and children fromcreateChildEntity.Configis constructed first inKreatures.__init__so the world can be given the value.src/stats/__init__ copy.pyhas been deleted. It was byte-identical tosrc/stats/__init__.py, is not importable (the space makes the name invalid as a module identifier), and is referenced by nothing.Test plan
python3 -m pytest --verbose -vv --cov=src --cov-report=term-missing— 66 passed (65 before; one test added).python3 -m compileall -q src— clean.tests/test_lag_prevention.py::TestPerformanceOptimizations::test_config_log_cap_is_the_only_source_of_truthwas added as the regression guard: aConfigwithentityLogMaxSize = 7is injected intoKreatures, and every entity — player, starters, spawned, child — is asserted to carrylog.maxlen == 7, with enforcement checked by overflowing the log. This test fails against the previous code, where entities built without the config value kept the stale default of 50.maxLogSizewere updated to set the cap at construction instead; their assertions were kept intact rather than weakened.src/kreatures.pywas run locally with mocked input; the simulation completed and printed its summary, confirming thatlogbeing adequefrom construction is compatible with thelog[0]/del log[0]handling inKreatures.run.Deferred this cycle
The remaining open issues were not picked up, for these reasons:
COPYRIGHT.mdis on this project's do-not-auto-merge list, so it needs explicit human sign-off rather than an autonomous cycle..github/copilot-instructions.mdis agent-loaded configuration and requires separate explicit authorization before being edited.Notes
Pre-existing
blackdrift in the touched files (single-quoted strings, trailing whitespace on blank lines) was deliberately left alone so that this diff stays scoped to the two issues; it belongs in its own formatting-only sweep.Closes #40
Closes #43
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).