Skip to content

Fix CameraAttributes/Environment handling in PhantomCamera3D to enables runtime animations, and prevents Git noise#620

Open
Mustfa-IT wants to merge 2 commits intoramokz:mainfrom
Mustfa-IT:fix-runtime-attributes
Open

Fix CameraAttributes/Environment handling in PhantomCamera3D to enables runtime animations, and prevents Git noise#620
Mustfa-IT wants to merge 2 commits intoramokz:mainfrom
Mustfa-IT:fix-runtime-attributes

Conversation

@Mustfa-IT
Copy link
Contributor

This pull request introduces a copy-write pattern to reliably detect and apply changes to camera attributes and environment resources in phantom_camera_host.gd, addressing limitations in Godot's change signal system. The changes ensure that updates to these resources are properly recognized both in the editor and at runtime, particularly for animations, by tracking references and property hashes.

Change detection and copy-write logic for camera resources:

  • Added tracking variables (_last_applied_attributes, _last_applied_environment, and their hashes) to monitor the last applied camera attributes and environment, enabling detection of reference and property changes.
  • Implemented logic in _pcam_follow to detect changes in camera attributes and environment by comparing references and property hashes, applying updates via direct assignment in the editor and duplication at runtime to avoid unnecessary Git changes and ensure animation responsiveness.

Resource reset on camera switch:

  • Reset the tracked attributes and environment (and their hashes) whenever a new active phantom camera is assigned, ensuring accurate change detection when switching cameras.
Screen.Recording.2026-01-17.160204.mp4

…detect changes to fix the current Godot Limitation
@Mustfa-IT
Copy link
Contributor Author

this aimed to Fix
#388
#368

@ramokz
Copy link
Owner

ramokz commented Jan 19, 2026

Can confirm it works with editing the AnimationPlayer, which is great!
I have not been fully able to reproduce the duplicate resource ID git noise issue in the stable branch, so that might have been fixed on its own?

The only thing about this PR is that it does also introduce a potential user issue where you can modify the attribute value in the Camera3D node, which then also changes the PCam3D's attribute. If that resource is a saved or shared resource, it's easy to unknowingly cause cascading changes as the user had not manually assigned the resource to the Camera3D. Have set it up intentionally so that a user could not accidentally change a PCam3D's indirectly by modifying the Camera3D node. E.g., if you try to change a Camera3D property like V Offset or Current you get a warning output.

Think we should still have safeguards so users don't mistakenly change other resources if we can avoid it, but open to suggestions.

@Mustfa-IT
Copy link
Contributor Author

The only thing about this PR is that it does also introduce a potential user issue where you can modify the attribute value in the Camera3D node, which then also changes the PCam3D's attribute. If that resource is a saved or shared resource, it's easy to unknowingly cause cascading changes as the user had not manually assigned the resource to the Camera3D. Have set it up intentionally so that a user could not accidentally change a PCam3D's indirectly by modifying the Camera3D node. E.g., if you try to change a Camera3D property like V Offset or Current you get a warning output.

yeah i had that in mind when i did this but i don't have any clean way of doing this at the moment
i try to do some safe guard like the H offset and the other.

@Mustfa-IT
Copy link
Contributor Author

Sorry I have been quite busy lately.

Hopefully the day after tomorrow I will be pushing a commit for this.

@ramokz
Copy link
Owner

ramokz commented Jan 26, 2026

There's no rush! Have been busy with things myself, so I likely won't be able to review it properly until towards the end of the week in any case :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants