Refactor stepping & units; add polygon utilities (v2.0.0)#2
Refactor stepping & units; add polygon utilities (v2.0.0)#2GoetzDeBouville merged 7 commits intomainfrom
Conversation
…mand units explicit - move fixed-timestep ownership (accumulator/maxSubSteps/fixed dt) to PhysicsSimulationLoop - change internal PhysicsWorldEngine.step signature to accept deltaSeconds + StepConfig + SolverIterations - make JVM PhysicsWorldEngine.step execute exactly one world.step(...) per call (or 0 when paused) - remove engine-side accumulator/clamp/substep loop and related state - rename command fields: - EnqueueImpulse: impulseX/impulseY -> impulseXPx/impulseYPx - EnqueueVelocity: velocityX/velocityY -> velocityXPxPerSec/velocityYPxPerSec - update all command call sites and engine conversions to the new field names - update KDoc in PhysicsCommand and PhysicsBoxState to explicitly state Px and Px/s units - clarify sleep callback semantics: parameter is isSleeping (true means sleeping)
update kdoc
make worldManifold as property remove NoOpEventSink
update docs update version
Общее описаниеОбновление включает переименование параметров физических команд (impulseX/Y → impulseXPx/impulseYPx), изменение сигнатуры метода PhysicsWorldEngine.step(), добавление новых утилит для полигональных форм, обновление версий зависимостей и рефакторинг системы событий физического движка. Изменения
Оценка сложности рецензирования🎯 3 (Moderate) | ⏱️ ~25 minutes Стихотворение
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/events/CollisionEvent.kt (1)
14-16: Уберите дублирующую формулировку про px-scale.На Line 14-16 повторяется одна и та же мысль, из‑за чего KDoc читается тяжелее. Лучше оставить одну формулировку.
Предлагаемая правка
- * - Treat this as a px-scaled magnitude suitable for UX effects (sound/haptics/FX thresholds), - * not as raw physics units. - * - Treat as px-scaled magnitude (UI scale), not N·s and not m·kg/s. + * - Treat this as a px-scaled magnitude suitable for UX effects (sound/haptics/FX thresholds), + * not as raw physics units (i.e., not N·s / kg·m/s).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/events/CollisionEvent.kt` around lines 14 - 16, In the KDoc for the CollisionEvent (the commented bullet lines describing px-scaling), remove the duplicated sentence and keep a single clear phrasing; e.g. retain one bullet such as "Treat as a px-scaled magnitude suitable for UX effects (sound/haptics/FX thresholds), not as raw physics units." Ensure you edit the KDoc block above the CollisionEvent declaration so only one px-scale bullet remains and formatting remains consistent.physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/PhysicsBoxState.kt (1)
209-215: Рекомендую унифицировать имена параметровenqueueVelocityс явными единицами.Сейчас KDoc уже говорит про Px/s, но сигнатура всё ещё
velocityX/velocityY. Для консистентности публичного API лучше перейти наvelocityXPxPerSec/velocityYPxPerSec.Предложенный diff
- fun enqueueVelocity( - key: Any, - velocityX: Float, - velocityY: Float, - ) { + fun enqueueVelocity( + key: Any, + velocityXPxPerSec: Float, + velocityYPxPerSec: Float, + ) { enqueueCommand( PhysicsCommand.EnqueueVelocity( key = key, - velocityXPxPerSec = velocityX, - velocityYPxPerSec = velocityY, + velocityXPxPerSec = velocityXPxPerSec, + velocityYPxPerSec = velocityYPxPerSec, ), ) }Also applies to: 220-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/PhysicsBoxState.kt` around lines 209 - 215, Rename the parameters in the public function enqueueVelocity from velocityX/velocityY to velocityXPxPerSec/velocityYPxPerSec to match the KDoc units; update the function signature in PhysicsBoxState.enqueueVelocity, all internal references inside that method, any overloaded variants or duplicate signatures (the other enqueueVelocity occurrences), and all call sites/tests to use the new names, and adjust the KDoc param tags if needed so the API and docs are consistent.physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/layout/PhysicsBox.kt (1)
112-114: Уберите дублирование сборки runtime-конфига для engine.На Line 133 повторно вычисляется
config.copy(step = state.stepConfig), хотя уже естьruntimeConfig. Лучше использовать один источник, чтобы избежать будущих расхождений.Предлагаемое упрощение
- config = config.copy(step = state.stepConfig), + config = runtimeConfig,Also applies to: 133-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/layout/PhysicsBox.kt` around lines 112 - 114, Уберите дублирование: вместо повторного вызова config.copy(step = state.stepConfig) в участке кода, где инициализируется engine (используя тот же выражение на Line 133), передавайте уже вычисленный runtimeConfig (значение, созданное через remember) — то есть замените повторное копирование на использование runtimeConfig при инициализации engine/вызове, чтобы runtimeConfig, config и state.stepConfig оставались единым источником правды.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gradle/libs.versions.toml`:
- Line 7: The packageVersion field currently set as packageVersion = "1.0.4"
must be bumped to the breaking-change release version from the PR: update
packageVersion to "2.0.0" (i.e., change the value of the packageVersion key) to
reflect the major semver bump and ensure any release metadata (changelog/release
tag) aligns with this version.
- Around line 10-13: В файле libs.versions.toml замените значение переменной
material3, потому что текущая "1.10.0-alpha05" отсутствует в Google Maven;
откройте запись material3 и установите корректную доступную версию (например
"1.5.0-alpha15" или ближайшую стабильную "1.4.0"), затем пересоберите, чтобы
убедиться, что зависимость разрешается корректно.
In
`@physicsbox/src/jvmMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsContactListener.kt`:
- Around line 20-22: PhysicsContactListener currently ignores sensor contacts
because beginContact/endContact are no-ops and postSolve only fires for impulse
contacts; implement sensor handling in beginContact(contact: Contact) and
endContact(contact: Contact) to detect contact.isSensor or
Contact.fixtureA/fixtureB.isSensor, build and dispatch the existing event
payload (same shape as postSolve collision events) for sensors, or alternatively
add explicit documentation to docs/events.md and the public API docs in
PhysicsContactListener noting that only impulse collisions from postSolve are
emitted and sensor contacts are excluded; update the code paths that rely on
sensor events (postSolve handlers) to use the new beginContact/endContact sensor
events if you choose implementation.
---
Nitpick comments:
In
`@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/events/CollisionEvent.kt`:
- Around line 14-16: In the KDoc for the CollisionEvent (the commented bullet
lines describing px-scaling), remove the duplicated sentence and keep a single
clear phrasing; e.g. retain one bullet such as "Treat as a px-scaled magnitude
suitable for UX effects (sound/haptics/FX thresholds), not as raw physics
units." Ensure you edit the KDoc block above the CollisionEvent declaration so
only one px-scale bullet remains and formatting remains consistent.
In
`@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/layout/PhysicsBox.kt`:
- Around line 112-114: Уберите дублирование: вместо повторного вызова
config.copy(step = state.stepConfig) в участке кода, где инициализируется engine
(используя тот же выражение на Line 133), передавайте уже вычисленный
runtimeConfig (значение, созданное через remember) — то есть замените повторное
копирование на использование runtimeConfig при инициализации engine/вызове,
чтобы runtimeConfig, config и state.stepConfig оставались единым источником
правды.
In
`@physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/PhysicsBoxState.kt`:
- Around line 209-215: Rename the parameters in the public function
enqueueVelocity from velocityX/velocityY to velocityXPxPerSec/velocityYPxPerSec
to match the KDoc units; update the function signature in
PhysicsBoxState.enqueueVelocity, all internal references inside that method, any
overloaded variants or duplicate signatures (the other enqueueVelocity
occurrences), and all call sites/tests to use the new names, and adjust the KDoc
param tags if needed so the API and docs are consistent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
docs/shapes.mdgradle/libs.versions.tomlphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/PhysicsBoxState.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/PhysicsCommand.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsEventSink.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsWorldEngine.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/events/CollisionEvent.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/layout/PhysicsBox.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/layout/PhysicsSimulationLoop.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/physicsbody/PhysicsBody.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/physicsbody/PhysicsBodyCallbacks.ktphysicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/utils/PolygonShapeUtils.ktphysicsbox/src/jvmMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsContactListener.ktphysicsbox/src/jvmMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsWorldEngine.ktsharedUI/build.gradle.ktssharedUI/src/commonMain/kotlin/com/zinchenkodev/app/demos/BasicStackingDemoScreen.ktsharedUI/src/commonMain/kotlin/com/zinchenkodev/app/demos/BoundariesConfigDemoScreen.ktsharedUI/src/commonMain/kotlin/com/zinchenkodev/app/demos/PhysicsBoxStateDemoScreen.kt
💤 Files with no reviewable changes (3)
- sharedUI/build.gradle.kts
- physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/physicsbody/PhysicsBodyCallbacks.kt
- physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsEventSink.kt
| minSdk = "23" | ||
|
|
||
| packageVersion = "1.0.3" | ||
| packageVersion = "1.0.4" |
There was a problem hiding this comment.
Версия релиза не соответствует заявленному breaking-change PR.
На Line [7] стоит packageVersion = "1.0.4", но в описании PR указан релиз v2.0.0 с breaking changes. Это создаст некорректную публикацию/семвер-сигнал для потребителей.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gradle/libs.versions.toml` at line 7, The packageVersion field currently set
as packageVersion = "1.0.4" must be bumped to the breaking-change release
version from the PR: update packageVersion to "2.0.0" (i.e., change the value of
the packageVersion key) to reflect the major semver bump and ensure any release
metadata (changelog/release tag) aligns with this version.
| compose-multiplatform = "1.10.1" | ||
| material3 = "1.10.0-alpha05" | ||
| agp = "8.13.2" | ||
| androidx-activityCompose = "1.12.3" | ||
| kermit = "2.0.8" | ||
| androidx-activityCompose = "1.12.4" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Проверка compose-multiplatform 1.10.1 (Maven Central) ==="
curl -fsSL "https://repo1.maven.org/maven2/org/jetbrains/compose/compose-gradle-plugin/maven-metadata.xml" 2>/dev/null | grep -o '<version>1\.10\.1</version>' || echo "Версия 1.10.1 не найдена"
echo
echo "=== Проверка material3 1.10.0-alpha05 (Google Maven) ==="
curl -fsSL "https://dl.google.com/dl/android/maven2/androidx/compose/material3/material3/maven-metadata.xml" 2>/dev/null | grep -o '<version>1\.10\.0-alpha05</version>' || echo "Версия 1.10.0-alpha05 не найдена"
echo
echo "=== Проверка agp 8.13.2 (Google Maven) ==="
curl -fsSL "https://dl.google.com/dl/android/maven2/com/android/tools/build/gradle/maven-metadata.xml" 2>/dev/null | grep -o '<version>8\.13\.2</version>' || echo "Версия 8.13.2 не найдена"
echo
echo "=== Проверка androidx-activityCompose 1.12.4 (Google Maven) ==="
curl -fsSL "https://dl.google.com/dl/android/maven2/androidx/activity/activity-compose/maven-metadata.xml" 2>/dev/null | grep -o '<version>1\.12\.4</version>' || echo "Версия 1.12.4 не найдена"Repository: GoetzDeBouville/PhysicsBox
Length of output: 409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Поиск доступных версий material3 (Google Maven) ==="
curl -fsSL "https://dl.google.com/dl/android/maven2/androidx/compose/material3/material3/maven-metadata.xml" 2>/dev/null | grep -o '<version>[^<]*</version>' | tail -20 || echo "Не удалось получить метаданные"
echo
echo "=== Проверка специфично для alpha версий ==="
curl -fsSL "https://dl.google.com/dl/android/maven2/androidx/compose/material3/material3/maven-metadata.xml" 2>/dev/null | grep -i 'alpha' || echo "Alpha версии не найдены"Repository: GoetzDeBouville/PhysicsBox
Length of output: 3775
Версия material3 "1.10.0-alpha05" недоступна в Google Maven и вызовет ошибку сборки.
Проверка показала:
- compose-multiplatform 1.10.1 ✓ доступна
- agp 8.13.2 ✓ доступна
- androidx-activityCompose 1.12.4 ✓ доступна
- material3 1.10.0-alpha05 ✗ не существует в репозитории
В Google Maven доступны версии material3 от 1.0.0-alpha* до 1.5.0-alpha15, но версии 1.10.0-alpha05 нет. Измените версию material3 на существующую (например, 1.5.0-alpha15 или ближайшую стабильную 1.4.0).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gradle/libs.versions.toml` around lines 10 - 13, В файле libs.versions.toml
замените значение переменной material3, потому что текущая "1.10.0-alpha05"
отсутствует в Google Maven; откройте запись material3 и установите корректную
доступную версию (например "1.5.0-alpha15" или ближайшую стабильную "1.4.0"),
затем пересоберите, чтобы убедиться, что зависимость разрешается корректно.
| override fun beginContact(contact: Contact) = Unit | ||
|
|
||
| override fun endContact(contact: Contact) = Unit |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PhysicsContactListener.kt" -type fRepository: GoetzDeBouville/PhysicsBox
Length of output: 159
🏁 Script executed:
find . -type f -name "*.kt" | grep -i contact | head -20Repository: GoetzDeBouville/PhysicsBox
Length of output: 159
🏁 Script executed:
git ls-files | grep -i listener | head -20Repository: GoetzDeBouville/PhysicsBox
Length of output: 157
🏁 Script executed:
cat -n ./physicsbox/src/jvmMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsContactListener.ktRepository: GoetzDeBouville/PhysicsBox
Length of output: 2673
🌐 Web query:
JBox2D ContactListener postSolve sensor contacts impulse
💡 Result:
In JBox2D (Box2D), sensor contacts do not produce collision response, so there’s no solver impulse to report.
PostSolve(contact, impulse)is only called for contacts that are “touching, solid, and awake.” Sensors are not “solid,” so you should not expectpostSolvefor sensor-vs-anything contacts, nor meaningful impulse values. [1]PreSolveis not called for sensors (same reason: no collision response step to configure). [1]- Sensors “do not generate contact points” and are intended to be handled via
BeginContact/EndContact(orcontact.isTouching()). [2] - Therefore,
ContactImpulseis effectively “not applicable” for sensors; if you need an “impact strength” for a sensor overlap, you must compute it yourself (e.g., using relative velocity / momentum atBeginContact). [2][3]
Practical rule:
- Use
BeginContact/EndContactfor sensors. - Use
PostSolve+ContactImpulseonly for non-sensor (solid) contacts.
Sources: [1][2][3]
🏁 Script executed:
find . -type f \( -name "*.kt" -o -name "*.java" \) | xargs grep -l "sensor" | head -10Repository: GoetzDeBouville/PhysicsBox
Length of output: 313
🏁 Script executed:
find . -type f -name "*Test*" -o -name "*test*" | grep -i contact | head -10Repository: GoetzDeBouville/PhysicsBox
Length of output: 52
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "sensor" | head -5Repository: GoetzDeBouville/PhysicsBox
Length of output: 313
🏁 Script executed:
rg "PhysicsContactListener" --type kt -A 3 -B 3Repository: GoetzDeBouville/PhysicsBox
Length of output: 96
🏁 Script executed:
rg "PhysicsContactListener" -A 3 -B 3Repository: GoetzDeBouville/PhysicsBox
Length of output: 1770
🏁 Script executed:
rg "beginContact|endContact" --type-list | head -5Repository: GoetzDeBouville/PhysicsBox
Length of output: 459
🏁 Script executed:
rg "sensor" --max-count 20Repository: GoetzDeBouville/PhysicsBox
Length of output: 2140
🏁 Script executed:
rg "CollisionEvent" -A 2 -B 2Repository: GoetzDeBouville/PhysicsBox
Length of output: 14083
🏁 Script executed:
find . -type f -name "*.md" | xargs grep -l "sensor\|contact\|lifecycle" 2>/dev/null | head -5Repository: GoetzDeBouville/PhysicsBox
Length of output: 108
🏁 Script executed:
cat -n ./docs/events.mdRepository: GoetzDeBouville/PhysicsBox
Length of output: 1790
🏁 Script executed:
cat -n ./physicsbox/src/commonMain/kotlin/dev/zinchenko/physicsbox/events/CollisionEvent.ktRepository: GoetzDeBouville/PhysicsBox
Length of output: 2049
Потеря событий сенсорных контактов из-за полагания только на postSolve.
В JBox2D postSolve вызывается только для solid контактов с физическим ответом (когда есть impulse). Сенсорные контакты не генерируют impulses, поэтому postSolve для них никогда не срабатывает. Текущая реализация полностью теряет события таких контактов, так как beginContact и endContact пусты.
Требуется либо:
- Реализовать обработку сенсорных контактов в
beginContact/endContact, либо - Явно задокументировать в API и в
docs/events.md, что сообщаются только импульсные столкновения изpostSolveи сенсорные контакты исключены.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@physicsbox/src/jvmMain/kotlin/dev/zinchenko/physicsbox/engine/PhysicsContactListener.kt`
around lines 20 - 22, PhysicsContactListener currently ignores sensor contacts
because beginContact/endContact are no-ops and postSolve only fires for impulse
contacts; implement sensor handling in beginContact(contact: Contact) and
endContact(contact: Contact) to detect contact.isSensor or
Contact.fixtureA/fixtureB.isSensor, build and dispatch the existing event
payload (same shape as postSolve collision events) for sensors, or alternatively
add explicit documentation to docs/events.md and the public API docs in
PhysicsContactListener noting that only impulse collisions from postSolve are
emitted and sensor contacts are excluded; update the code paths that rely on
sensor events (postSolve handlers) to use the new beginContact/endContact sensor
events if you choose implementation.
Description:
Breaking changes:
Summary by CodeRabbit
Примечания к выпуску
Новые функции
Документация
Улучшения
Версия: 1.0.4