-
-
Notifications
You must be signed in to change notification settings - Fork 0
Dev #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis set of changes refines the handling of scale components in the camera rendering logic by normalizing zero z-scale values to one, preventing early exits or skips. The Changes
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
include/Silver.hpp
(3 hunks)src/SilverCamera.cpp
(2 hunks)src/SilverSpriteRendering.cpp
(2 hunks)src/main.cpp
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main.cpp (2)
include/Silver.hpp (1)
Hold
(456-456)src/Silver.cpp (2)
Hold
(544-547)Hold
(544-544)
🔇 Additional comments (8)
include/Silver.hpp (2)
144-144
: Default member initialization changed to match constructorThis change ensures consistency with the updated constructor initialization on line 108, properly synchronizing the default value for
useRelativePivot
across all initialization paths.
202-202
: Parameter type change improves rotation precisionChanging parameter types from
int
todouble
for theRotatePoint
method enables more precise floating-point calculations during rotation, which is particularly valuable for achieving smooth rotations with fractional positions.This change should be implemented consistently in any corresponding method implementation.
src/SilverCamera.cpp (2)
160-162
: Improved handling of zero z-scale valuesInstead of skipping rendering when z-scale is zero, the code now normalizes it to 1 and only checks x and y components for early return. This allows objects with zero z-scale to be properly rendered instead of being incorrectly filtered out.
273-274
: Consistent handling of zero scale componentsThis change normalizes zero z-scale to 1 while preserving the existing behavior for zero x or y scale components. It allows objects to be visible even when the z component of scale is zero, while still skipping objects with zero width or height.
The explicit floating-point comparison (
0.0f
) also improves code clarity and precision.src/main.cpp (1)
4-15
: Stack allocation instead of heap improves performance and simplifies codeThe code has been refactored to use stack-allocated Actor objects instead of shared pointers, which simplifies memory management and improves performance. It also uses
StartVideo()
instead ofRenderFrame()
, which suggests a shift to a more persistent rendering approach.The commented-out alignment call suggests the new alignment feature from SilverSpriteRendering.cpp is ready but not yet utilized.
This approach eliminates potential memory leaks and reduces overhead from reference counting in shared pointers.
src/SilverSpriteRendering.cpp (3)
135-135
: Method signature updated to match header declarationThe
RotatePoint
method parameters are nowdouble
instead ofint
, matching the updated declaration in Silver.hpp. This enables more precise rotation calculations with floating-point values.
355-365
: Improved shape processing and size calculationThe
setShape
method has been refactored to:
- Apply ANSI extraction after Markdown processing
- Calculate size using the
GetSize()
method instead of manually counting- Simplify the code flow
These changes make the shape processing more maintainable and consistent, ensuring that size calculations are centralized in one method.
367-411
: New alignment feature adds improved text positioningThe new
alignShapeTo(double align)
method provides horizontal alignment capabilities for sprite shapes, which enhances UI and text rendering possibilities. The implementation correctly:
- Clamps the alignment factor between 0 and 1
- Calculates required padding for each line
- Properly handles ANSI escape codes alignment
- Updates all relevant internal state
This is a valuable addition that increases the flexibility of the text rendering system.
Note that there's a commented-out usage of this method in main.cpp, suggesting it's ready for use but not yet actively applied.
include/Silver.hpp
Outdated
@@ -105,7 +105,7 @@ class SpriteRenderer : public Component { | |||
setShape(newShape); | |||
cleanShape = newShape; | |||
|
|||
useRelativePivot = false; | |||
useRelativePivot = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default pivot behavior changed from absolute to relative
The default value for useRelativePivot
was changed from false
to true
, which means sprites will now use relative pivot positioning by default (pivot as a percentage of sprite size) instead of absolute positioning.
This change offers better flexibility when handling sprites of different sizes, but could affect existing code expecting the previous default behavior.
#!/bin/bash
# Search for any instances where SpriteRenderer is created without explicitly setting useRelativePivot
# to check for potential impact of this change
echo "Searching for SpriteRenderer instantiations that might be affected by the default pivot change:"
rg -A 3 "new SpriteRenderer\(" --type cpp
rg -A 3 "AddComponent<SpriteRenderer>\(" --type cpp
rg -A 3 "= SpriteRenderer\(" --type cpp
Summary by CodeRabbit