Skip to content
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

Break apart AddToManagers in generated code to allow factories to generate first #1172

Open
vchelaru opened this issue Sep 4, 2023 · 0 comments
Labels
bug huge This issue may take many days or even weeks of work.

Comments

@vchelaru
Copy link
Owner

vchelaru commented Sep 4, 2023

Summary

The AddToManagers method should not be a virtual method. Instead, it should be called in the Base class, and then all other methods can be virtual and called by the base implementation so that it can control order.

Example problem

This is in context of Screens, but this change is probably okay to make on Entities.

Currently ScreenManager calls AddToManagers on a Screen, which is a virtual method. This gets overridden by derived Screens which do their addition first, then allowing the base screen to do its addition. I'm not sure exactly why but I suppose there is probably logic that requires the derived to do this first.

The problem with this is that the AddToManagers method is where Factories call AddList. Until AddList is called, if a factory instantiates an object, that object will not get added to the list. This can result in entities being created (due to derived code being called first), and those entities not being added to their lists.

For example, consider the following situation:

GameScreen is the base class so it is responsible for Initializing factories. Level1 (inheriting from GameScreen) creates an entity such as EnemyVehicle. EnemyVehicle may include its own entities, and these entities may be initialized in EnemyVehicle CustomInitialize. Since EnemyVehicle is created in Level1, its CustomInitialize is called before the base GameScreen has a chance to initialize its factories. Any entity that EnemyVehicle creates will not be owned by the factory.

Detailed Fix

As mentioned above, AddToManagers should no longer be a virtual method. It should be a method implemented only in the base object. Then it should provide virtual methods that overrides can call.

For example, this is the current GameScreen.Generated AddToManagers:

        public override void AddToManagers()
        {
            mAccumulatedPausedTime = TimeManager.CurrentTime;
            mTimeScreenWasCreated = FlatRedBall.TimeManager.CurrentTime;
            GameScreenGum.AddToManagers(); FlatRedBall.FlatRedBallServices.GraphicsOptions.SizeOrOrientationChanged += RefreshLayoutInternal;
            InitializeFactoriesAndSorting();
            Player1.AddToManagers(mLayer);
            ScreenTopBottomShapes.AddToManagers();
            FlatRedBall.SpriteManager.AddPositionedObject(CameraControllingEntityInstance); CameraControllingEntityInstance.Activity();
            mScreenRectangleCollection.AddToManagers();
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(PlayerList, FlatRedBall.Math.Axis.Y, 16f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(EnemyList, FlatRedBall.Math.Axis.Y, 32f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(StaticDestroyableObjectList, FlatRedBall.Math.Axis.Y, 12.8125f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(BulletList, FlatRedBall.Math.Axis.Y, 32f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(PickupContainerList, FlatRedBall.Math.Axis.Y, 32f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(EnemyBulletList, FlatRedBall.Math.Axis.Y, 32f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(DestructibleObstacleList, FlatRedBall.Math.Axis.Y, 29.714207f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(FriendlyEnemyList, FlatRedBall.Math.Axis.Y, 32f, true);
            FlatRedBall.Math.Collision.CollisionManager.Self.Partition(CheckpointList, FlatRedBall.Math.Axis.Y, 66.67919f, true);
            FlatRedBall.TileEntities.TileEntityInstantiator.CreateEntitiesFrom(Map);
            base.AddToManagers();
            AddToManagersBottomUp();
            BeforeCustomInitialize?.Invoke();
            CustomInitialize();
        }

This should instead be like this:

public void AddToManagers()
{
    mAccumulatedPausedTime = TimeManager.CurrentTime;
    mTimeScreenWasCreated = FlatRedBall.TimeManager.CurrentTime;
    GameScreenGum.AddToManagers(); FlatRedBall.FlatRedBallServices.GraphicsOptions.SizeOrOrientationChanged += RefreshLayoutInternal;

    InitializeFactoriesAndSorting();

    // This replaces the bulk of the method:
    AddToManagersTopDown();

    AddToManagersBottomUp();
    BeforeCustomInitialize?.Invoke();
    CustomInitialize();
}

protected virtual void InitializeFactoriesAndSorting()
{
    // .....
}

protected virtual void AddToManagersTopDown()
{
    // ....
}

// and so on

This problem was encountered in Cranky Chibi Cthulhu when an enemy was attempting to create another enemy in its CustomInitialize. It's a bigger issue so I'm logging this here without solving it for now, and I'm going to work around it.

@vchelaru vchelaru added bug huge This issue may take many days or even weeks of work. labels Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug huge This issue may take many days or even weeks of work.
Projects
None yet
Development

No branches or pull requests

1 participant