Skip to content

Comments

Animator 배치 스케줄 계약 강제화 및 커스텀 애니메이터 문서화#6

Open
shlifedev wants to merge 1 commit intomainfrom
codex/modify-floatingtextanimator-and-related-files
Open

Animator 배치 스케줄 계약 강제화 및 커스텀 애니메이터 문서화#6
shlifedev wants to merge 1 commit intomainfrom
codex/modify-floatingtextanimator-and-related-files

Conversation

@shlifedev
Copy link
Owner

Motivation

  • 커스텀 FloatingTextAnimator가 메인스레드에서 느린 managed 루프에 빠지는 것을 방지하고, 배치(Burst) 경로 미구현을 런타임에 빠르게 감지하기 위해 변경했습니다.
  • 개발 빌드/에디터에서만 경고를 1회만 출력해 로그 스팸을 줄이고, UNITY_ASSERTIONS 환경에서는 assert로 구현 누락을 즉시 발견하도록 합니다.

Description

  • Packages/.../Runtime/FloatingTextAnimator.cs: 기본 구현의 ScheduleEvaluateBatchUNITY_ASSERTIONS 기반 런타임 assert를 추가해 미구현 시 실패를 노출하고, DEVELOPMENT_BUILD || UNITY_EDITOR 조건으로 관리되는 한 번만 출력되는 경고 로그(Debug.LogWarning)를 추가했습니다.
  • Packages/.../Runtime/FloatingTextManager.cs: _animator.ScheduleEvaluateBatch(...) 호출 직전 Assert.IsNotNull(_animator)를 추가하고 호출 의도를 설명하는 주석을 보강했습니다.
  • Packages/.../Runtime/DefaultFloatingTextAnimator.cs: 기본 구현이 배치 경로(예: IJobParallelFor / Burst) 패턴임을 명시하는 주석을 추가했습니다.
  • README.md: 커스텀 animator 작성 규칙 섹션을 추가해 Evaluate(...)/ScheduleEvaluateBatch(...) 역할, ScheduleEvaluateBatch의 필수성(Burst job 제공) 및 fallback 동작(경고/assert 정책)을 문서화했습니다.

Testing

  • 코드 스타일/문법 검사: git diff --check를 실행했으며 에러 없음(성공).
  • 키워드/변경 검증: rg -n "ScheduleEvaluateBatch|Evaluate\(|_animator.ScheduleEvaluateBatch|UNITY_ASSERTIONS"로 관련 위치를 확인했고 의도한 파일들(FloatingTextAnimator.cs, FloatingTextManager.cs, DefaultFloatingTextAnimator.cs, README.md)이 수정된 것을 확인했습니다 (성공).
  • 변경사항 커밋 완료: 변경을 커밋했으며 로컬 커밋 해시가 생성됨(성공).
  • Unity 에디터/플레이모드 자동화 테스트는 CLI 환경 제약으로 수행하지 못했으므로 런타임 동작(Assert/로그 출력, Burst job 스케줄링 통합)은 에디터에서 재현하여 검증해야 합니다.

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce52cfba5c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +40 to +42
Assert.IsTrue(false,
$"[{nameof(FloatingTextAnimator)}] {GetType().Name} is using managed fallback in {nameof(ScheduleEvaluateBatch)}(). " +
"Override ScheduleEvaluateBatch() and provide a Burst-compatible job for production use.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate fallback assertion to avoid per-frame assertion storms

The new fallback path asserts unconditionally on every ScheduleEvaluateBatch call in UNITY_ASSERTIONS builds, so any custom animator that doesn’t override this method now triggers repeated assertion failures every frame; in Editor configurations where assertions raise exceptions this can also abort the fallback execution and prevent animation output for that frame. This contradicts the intended "warn once" behavior and makes development runs noisy/unusable until fixed, so the assert should be emitted once (or downgraded) instead of firing on every update.

Useful? React with 👍 / 👎.

@shlifedev
Copy link
Owner Author

PR 평가: 7/10 ✅

타당한 이유

  1. 계약(Contract) 명시화: FloatingTextAnimator.ScheduleEvaluateBatch()의 기본 구현에 assert와 경고를 추가하여, 커스텀 애니메이터 작성 시 반드시 Burst Job 기반 override를 제공해야 한다는 계약을 코드 수준에서 강제합니다.

  2. 스마트한 경고 전략: DEVELOPMENT_BUILD || UNITY_EDITOR 조건에서만 경고를 출력하고, s_loggedManagedFallbackWarning static 플래그로 1회만 출력하여 로그 스팸을 방지합니다. UNITY_ASSERTIONS 환경에서는 assert로 즉시 실패하여 구현 누락을 빠르게 감지합니다.

  3. 문서화: README에 커스텀 애니메이터 작성 규칙 섹션을 추가하여, Evaluate()ScheduleEvaluateBatch()의 역할과 필수성을 명확히 문서화했습니다. DefaultFloatingTextAnimator의 주석도 보강되어 코드 내 가이드가 충실합니다.

  4. Null 안전성: FloatingTextManager에서 _animator.ScheduleEvaluateBatch() 호출 전 Assert.IsNotNull(_animator)를 추가하여 null 참조 크래시를 사전에 방지합니다.

참고 사항

  • Assert.IsTrue(false, ...) 패턴은 약간 관례적이지 않으므로, Debug.Assert(false, ...) 또는 별도 커스텀 assert를 고려할 수 있습니다.
  • DefaultFloatingTextAnimator가 이미 올바른 구현을 제공하므로, 이 변경은 주로 서드파티 커스텀 애니메이터 작성자를 위한 안전망입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant