Add excrement spawning system for living entities to improve ranching with CI pipeline#238
Add excrement spawning system for living entities to improve ranching with CI pipeline#238Copilot wants to merge 6 commits into
Conversation
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
dmccoystephenson
left a comment
There was a problem hiding this comment.
@copilot Great job, now add a simple CI pipeline that runs the unit tests
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Added a GitHub Actions CI pipeline in commit |
|
@copilot Expand unit tests |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Expanded unit tests significantly in commit |
There was a problem hiding this comment.
Pull request overview
This PR adds an excrement spawning system where living entities periodically drop excrement that transforms into grass after aging, making ranching more sustainable. It also significantly expands test coverage from 74 to 107 tests (+45%) and introduces a CI pipeline using GitHub Actions.
Changes:
- Living entities spawn excrement every 5 minutes when moving
- Excrement automatically converts to grass after 10 minutes
- Comprehensive test suite expansion covering config, world, screen, UI, and library components
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/entity/excrement.py | New entity class for excrement with age tracking |
| src/entity/living/livingEntity.py | Added excrement spawning cooldown tracking and logic |
| src/world/room.py | Excrement spawning during movement and aging conversion to grass |
| src/screen/worldScreen.py | Integration of excrement aging in main game loop |
| assets/images/excrement.png | Visual asset for excrement entity |
| .github/workflows/ci.yml | New CI pipeline for automated testing |
| tests/entity/test_excrement.py | Tests for excrement entity behavior |
| tests/entity/living/test_livingEntity.py | Enhanced tests with excrement spawning coverage |
| tests/entity/test_drawableEntity.py | Additional tests for drawable entity functionality |
| tests/config/test_config.py | Comprehensive configuration testing |
| tests/world/test_tickCounter.py | Tests for tick counter save/load and timing |
| tests/world/test_roomType.py | Room type constant validation |
| tests/screen/test_screenType.py | Screen type constant validation |
| tests/ui/test_status.py | Status UI component testing |
| tests/mapimage/test_mapImageUpdater.py | Map image updater functionality tests |
| tests/lib/pyenvlib/test_entity.py | Base entity class testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.targetEnergy = energy | ||
| self.tickCreated = tickCreated | ||
| self.tickLastReproduced = None | ||
| self.tickLastExcrement = None |
There was a problem hiding this comment.
The tickLastExcrement field is not included in the save/load logic for LivingEntity. In roomJsonReaderWriter.py, the generateJsonForEntity method (lines 102-118) needs to include tickLastExcrement in the serialization for LivingEntity instances, and the generateEntityFromJson method (lines 176-228) needs to restore this value when loading. Without this, living entities will lose their excrement cooldown state when a game is saved and loaded, potentially causing incorrect spawning behavior.
| def __init__(self, tickCreated): | ||
| DrawableEntity.__init__(self, "Excrement", "assets/images/excrement.png") | ||
| self.solid = False | ||
| self.tickCreated = tickCreated |
There was a problem hiding this comment.
The Excrement entity is not handled in the inventory save/load logic. While excrement is not meant to be pickable (it's non-solid), if the inventory system is ever extended to support it, or if there's any edge case where excrement could end up in inventory, the inventoryJsonReaderWriter.py file should include handling for Excrement entities similar to how it handles other entity types.
| # check if entity should spawn excrement | ||
| if entity.shouldSpawnExcrement(tick): | ||
| # spawn excrement at the old location | ||
| excrement = Excrement(tick) | ||
| self.addEntityToLocation(excrement, location) | ||
| entity.setTickLastExcrement(tick) |
There was a problem hiding this comment.
There are no tests for the excrement spawning logic in the moveLivingEntities method. The new code that spawns excrement when entities move (lines 122-127) should have tests to verify that excrement is spawned at the correct location (the old location before movement) and that the tickLastExcrement is properly updated. Consider adding integration tests that verify excrement spawning during entity movement.
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Python 3.12 | ||
| uses: actions/setup-python@v4 |
There was a problem hiding this comment.
The GitHub Actions workflow uses actions/setup-python@v4, but version 5 is available and is the current recommended version. Consider updating to @v5 to use the latest features and improvements.
| uses: actions/setup-python@v4 | |
| uses: actions/setup-python@v5 |
| # Create a temporary PNG file for testing | ||
| with tempfile.NamedTemporaryFile(suffix='.png', delete=False) as temp_file: | ||
| # Create a simple 1x1 pixel PNG image | ||
| temp_surface = pygame.Surface((1, 1)) | ||
| temp_surface.fill((255, 255, 255)) | ||
| pygame.image.save(temp_surface, temp_file.name) | ||
| temp_path = temp_file.name | ||
|
|
||
| try: | ||
| drawableEntity = DrawableEntity("test", temp_path) | ||
| image = drawableEntity.getImage() | ||
|
|
||
| # Verify that getImage returns a pygame Surface | ||
| assert isinstance(image, pygame.Surface) | ||
| assert image.get_width() == 1 | ||
| assert image.get_height() == 1 | ||
| finally: | ||
| # Clean up the temporary file | ||
| if os.path.exists(temp_path): |
There was a problem hiding this comment.
The test_get_image function uses pygame.Surface and pygame.image.save without first calling pygame.init(). While this might work in some environments, it's best practice to initialize pygame before using its functionality. Consider adding pygame.init() at the start of the test and pygame.quit() in the finally block to ensure proper initialization and cleanup.
| # Create a temporary PNG file for testing | |
| with tempfile.NamedTemporaryFile(suffix='.png', delete=False) as temp_file: | |
| # Create a simple 1x1 pixel PNG image | |
| temp_surface = pygame.Surface((1, 1)) | |
| temp_surface.fill((255, 255, 255)) | |
| pygame.image.save(temp_surface, temp_file.name) | |
| temp_path = temp_file.name | |
| try: | |
| drawableEntity = DrawableEntity("test", temp_path) | |
| image = drawableEntity.getImage() | |
| # Verify that getImage returns a pygame Surface | |
| assert isinstance(image, pygame.Surface) | |
| assert image.get_width() == 1 | |
| assert image.get_height() == 1 | |
| finally: | |
| # Clean up the temporary file | |
| if os.path.exists(temp_path): | |
| pygame.init() | |
| temp_path = None | |
| try: | |
| # Create a temporary PNG file for testing | |
| with tempfile.NamedTemporaryFile(suffix='.png', delete=False) as temp_file: | |
| # Create a simple 1x1 pixel PNG image | |
| temp_surface = pygame.Surface((1, 1)) | |
| temp_surface.fill((255, 255, 255)) | |
| pygame.image.save(temp_surface, temp_file.name) | |
| temp_path = temp_file.name | |
| drawableEntity = DrawableEntity("test", temp_path) | |
| image = drawableEntity.getImage() | |
| # Verify that getImage returns a pygame Surface | |
| assert isinstance(image, pygame.Surface) | |
| assert image.get_width() == 1 | |
| assert image.get_height() == 1 | |
| finally: | |
| pygame.quit() | |
| # Clean up the temporary file | |
| if temp_path is not None and os.path.exists(temp_path): |
| assert livingEntity.getEnergy() == 50 | ||
| assert livingEntity.getTargetEnergy() == 50 | ||
| assert livingEntity.getTickCreated() == 0 | ||
| assert livingEntity.getTickLastExcrement() == None |
There was a problem hiding this comment.
Testing for None should use the 'is' operator.
| assert livingEntity.getTickLastExcrement() == None | |
| assert livingEntity.getTickLastExcrement() is None |
| def test_excrement_tick_tracking(): | ||
| livingEntity = createLivingEntity() | ||
|
|
||
| assert livingEntity.getTickLastExcrement() == None |
There was a problem hiding this comment.
Testing for None should use the 'is' operator.
| def test_tick_last_reproduced(): | ||
| livingEntity = createLivingEntity() | ||
|
|
||
| assert livingEntity.getTickLastReproduced() == None |
There was a problem hiding this comment.
Testing for None should use the 'is' operator.
| initial_tick = tickCounter.getTick() | ||
|
|
There was a problem hiding this comment.
Variable initial_tick is not used.
| initial_tick = tickCounter.getTick() | |
| @@ -0,0 +1,76 @@ | |||
| from unittest.mock import Mock, MagicMock | |||
There was a problem hiding this comment.
Import of 'MagicMock' is not used.
| from unittest.mock import Mock, MagicMock | |
| from unittest.mock import Mock |
|
@copilot address comments |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All code review comments addressed in commit CI Improvements: Updated to actions/setup-python@v5, added SDL_VIDEODRIVER=dummy for headless pygame Save/Load: Added tickLastExcrement and Excrement entity handling to roomJsonReaderWriter with backward compatibility Code Quality: Extracted timing constants to class-level (EXCREMENT_COOLDOWN_TICKS, EXCREMENT_AGE_THRESHOLD_TICKS), fixed None comparisons to use New Tests: Added 5 comprehensive tests for excrement spawning and aging logic All 112 tests passing ✅ |
All Code Review Comments Addressed:
CI/Build Improvements:
Save/Load Enhancements:
tickLastExcrementto save/load logic in roomJsonReaderWriterExcremententity handling in generateEntityFromJsonCode Quality Improvements:
isinstead of==New Test Coverage:
Test Count: 112 tests total (+5 from previous 107)
All Tests Passing: ✅ 112/112 with comprehensive coverage
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.