Question about destroying Player components vs removing from Viewport

3D
1

Hello,

My design:

TPlayerManager (TComponent, Owner = nil)
└── TPlayer (TComponent, Owner = TPlayerManager)
├── TCastleBehavior (Owner = TPlayer)
└── TCastleScene (Owner = TPlayer)

  • Player is a TComponent
  • TCastleScene and TCastleBehavior are created with Owner = TPlayer
  • TCastleScene / behaviors are added to Viewport.Items

My question (design):

When I want to destroy a player, what is the correct approach?

  1. Just destroy TPlayer
    (because all children are owned by it, and I see that Viewport.Items.Count decreases correctly)

OR

  1. Manually remove all scenes / behaviors from Viewport.Items
    (Remove / RemoveDelayed) and then destroy TPlayer

The problem:

  • When I create many players (for example ~80 players),
  • and later destroy them,
  • I sometimes get an exception in:

procedure TFreeNotificationObserver.Notification(
AComponent: TComponent; Operation: TOperation);

  • The application shuts down.

If I temporarily wrap this method in an empty try..except, the game continues to run, and:

  • all children are removed from Viewport.Items
  • Viewport.Items.Count looks correct
  • no memory leak is reported

I want to fix this properly, not hide it with empty try..except.

Thank you!

2

Hi!

First of all, it would be hard to see real reasons without code, so if possible, please provide a reproducible example.

Second :

this sounds a bit weird to me. why would one split this into different schemas of “ownership”, why don’t you make your manager a TCastleTransform and put everything into it as your hierarchy (with standard Add method), and Player would also be a Transform, and finally you put this manager into Viewport.Items ? this way you don’t care about destroying of players, and if you need to remove, you just Remove().

Third - for such things in gamedev usually Pools are used, so you don’t really remove anything, but reuse objects from the Pool. Initial creation could be done with CastleComponentFactory which is also could be used for non-pool way, bc it is optimised internally for loads of object creation

Fourth, please check good example of dynamic creation/removal of objects in the castle examples:

1 Like
3

Thanks.

Creation and destruction of players in my game is already very fast, since models are loaded once at startup and afterwards players are created by cloning existing scenes. So performance-wise this part is not an issue for me.

In my case, Player is not only a visual object. It also owns networking ,database, logic, and background threads. Because of that, separating game logic (TPlayer as TComponent) from rendering (TCastleScene / behaviors) felt more natural for me.

At this stage of the project, the game has grown quite a lot, so changing the base design now is not really possible. Based on my understanding so far, I believe the general direction is correct, :slight_smile: and the main thing I need to improve is handling lifetime and ownership between logic components and visual objects in a safer way.

The “false exceptions” I observed were related to TExposedTransform instances.
In CGE, TCastleTransform used by TExposedTransform is created like this:

      { Create new TCastleTransform.
        Note
        - We connect to it only by Name, not any other internal reference.
          This is important, as we need to restore the connection
          e.g. after deserializing it from design file.
        - We use current Owner as the owner, not Self.
          This makes it work in case of editor or TransformLoad nicely.
          Intuitively, we don't want to manage the lifetime of these scenes
          here: the lifetime of them should be up to our owner.
      }
      TransformChild := TCastleTransform.Create(Owner);
      TransformChild.Name := TransformNamePascal;
      Add(TransformChild);

Because TExposedTransform is freed automatically when its owner (TPlayer)
is destroyed, the previous implementation of ChildFreeNotification that tried
to free or remove it was unnecessary. In my case, I now clear all exposed
transforms manually in TPlayer before destruction (ClearPlayerExposedTransforms).
which avoids executing:
procedure TCastleSceneCore.TExposedTransform.ChildFreeNotification(const Sender: TFreeNotificationObserver);
begin
// free this TExposedTransform instance, removing it from FExposeTransforms
// ParentScene.FExposedTransforms.Remove(Self);
end;

Actually, there is really no need to call ClearPlayerExposedTransforms.
TExposedTransform is freed automatically when its owner (TPlayer) is destroyed.

When running the game and pressing F8, the CGE debug overlay shows
TExposedTransform objects as “existing.”

However, this does not mean they are actually alive in memory — all references
in the viewport are already removed, as confirmed by Viewport.Items.Count,
which is always correct.

If I manually execute ClearPlayerExposedTransforms, then these objects immediately
disappear from the debug overlay. “F8”

I may make a video to demonstrate this.

The important thing now: 0 exceptions :slight_smile: :slight_smile:

4

Just a small correction to what I said earlier regarding Viewport.Items.Count.

Viewport.Items.Count does not count all child transforms in the hierarchy.
It only counts the top-level items added directly to the viewport.

Because of that, TExposedTransform instances that appear in the CGE debug overlay (F8) are real and still alive, not false positives.

The reason they are still visible after destroying TPlayer is :):

The scene was created/loaded from the editor
The exposed transforms belong to that scene
Their Owner is the Viewport (or scene owned by the viewport), not TPlayer

So when TPlayer is destroyed:
All references from the player side are removed
Viewport.Items.Count remains correct (it never counted those exposed child transforms anyway)
The debug overlay is also correct, because those TExposedTransform objects are still owned by the viewport and therefore not destroyed yet
When the scene itself is owned by a TComponent, and that component is destroyed, all children (including exposed transforms) are destroyed correctly. → just avoid calling
TCastleSceneCore.TExposedTransform.ChildFreeNotification

So in summary:

Viewport.Items.Count is always correct
The debug overlay is always correct

The remaining exposed transforms were not a CGE bug, but an ownership issue

5

Sorry for delay in answering. Reading the thread now, I understand it’s all solved :slight_smile: Just to be clear:

  • If you have a particular ownership relation that cause exceptions when freeing → submit a bug with a testcase to reproduce. The idea of using TComponent ownership is that it should “just work” in any circumstances. Of course, the simpler you make the relations → the easier, but we really should support all cases :slight_smile:

  • Things are automatically removed from necessary lists, parents, when they are being freed. So e.g. TCastleTransform, when freed, will remove itself from parent TCastleTransform .

  • Be wary of removing things when iterating over a given list. We have TCastleTransform.RemoveDelayed and TCastleApplicationProperties.FreeDelayed to help in these situations.

Hope this helps :slight_smile: If there’s any bug here for me to investigate, go ahead and submit a testcase, we’ll fix :slight_smile:

1 Like
6

I created a test that should trigger exceptions.
You can try creating and destroying the players 2–3 times, and the exception would normally be visible.
On my side, I fixed it by modifying the TCastleSceneCore.ExposeTransformsChange(Sender: TObject) procedure i made the new transforms owned by self instead of the default owner. After this change, there are no exceptions.

{ Create new TCastleTransform.
Note
- We connect to it only by Name, not any other internal reference.
This is important, as we need to restore the connection
e.g. after deserializing it from design file.
- We use current Owner as the owner, not Self.
This makes it work in case of editor or TransformLoad nicely.
Intuitively, we don’t want to manage the lifetime of these scenes
here: the lifetime of them should be up to our owner.
}
TransformChild := TCastleTransform.Create({Owner}Self);

https://drive.google.com/file/d/1c9aeZhRBR_RhS7SBfoQ2iwa4RXEHBh1K/view

7

Thanks!

I run the code using FPC 3.2.2 on Linux/x86_64 now (made a trivial code adjustment to build with FPC, THashSHA1.GetHashString(PointerToString64(Result)) → IntToStr(PtrUInt(Result))). Hm, admittedly I don’t reproduce bug, pressing Create + Destroy multiple times works OK.

But errors like dangling pointers may be ~random, depending on memory contents, compiler, optimizations. I’ll retry soon on Windows with latest FPC and/or Delphi.

1 Like
8

Thank you for reporting, this was a serious bug, our code had undefined behavior when freeing any scene with exposed transforms.

As a bonus, I realized our TCastleSceneCore.ExposeTransformsChange is unoptimal by recreating FExposedTransforms contents each time. Optimized in Optimize TCastleSceneCore.ExposeTransformsChange · castle-engine/castle-engine@cafaf99 · GitHub .

  • Tested on your testcase, with speedup 1.6->0.8, so 2x faster :slight_smile: Tested using our Timer to measure TPlayerManager.CreatePlayers time:
procedure TPlayerManager.CreatePlayers(Count: Integer; Parent: TCastleRootTransform);
var
  ....
  TimeStart: TTimerResult;
begin
  TimeStart := Timer;
  ...
  WritelnLog('Seconds passed: %f', [TimeStart.ElapsedTime]);
end;
2 Likes