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

Lifecycle class #767

Closed
originalfoo opened this issue Mar 2, 2020 · 3 comments · Fixed by #1077
Closed

Lifecycle class #767

originalfoo opened this issue Mar 2, 2020 · 3 comments · Fixed by #1077
Assignees
Labels
code cleanup Refactor code, remove old code, improve maintainability discussion Contains debate on certain topics technical Tasks that need to be performed in order to improve quality and maintainability
Milestone

Comments

@originalfoo
Copy link
Member

originalfoo commented Mar 2, 2020

Currently the lifecycle for TM:PE is completely obfuscated across multiple classes, it's impossible to see at a glance what is going on through the various stages, and it's getting even more confusing now that we're supporting hot reloads, etc.

In #764 I suggested briefly this: #764 (comment)

I think at some point we should consider creating a Lifecycle.cs that contains some sort of state engine for the mod lifecycle management (enabed -> loading -> unloading -> disabled) but that's something for a later date.

Kian then added this: #764 (comment)

I have thought of that but I can't see how it would work.

the only thing I can think of is:

  1. move all code from OnEnabled/OnDisabled, LoadingExtension.cs, and SerializeDataExtension.cs to lifecycle.cs
  2. then all OnEnabled/OnDisabled, LoadingExtension.cs, and SerializeDataExtension.cs do is to call appropriate functions of lifecycle.cs at appropriate times.

So lifeCycle.cs acts as a wrapper for IUserMod, LoadingExtension, and SerializableDataExtension . A bit ugly if you ask me.

The best thing to do is a mod for hot reload to patch CS so that it does not do weird stuff.

Even if we have mod to patch CS to not do weird stuff, we should still have a more coherent way of managing TMPE lifecycle.

Building on Kian's ideas, I suggest:

  1. Lifecycle class gets stub methods for the various states
  2. TrafficManagerMod and LoadingExtension (and anything else relevant) simply calls the applicable lifecycle methods
  3. Rather than filling lifecycle methods with bloated code, we should atomise funcitonality in to methods in some other class
  4. The lifecycle would then just call those other methods

The result would be a very concise lifecycle class and you could just read it to see exactly what steps happen in each stage of the lifecycle without getting bogged down in reams of code.

For example, enabled/disabled stuff in Lifecycle.cs could look something like (rough mockup):

// comment would note all the ways enabled can be triggered, such as:
// * mod autoenabler mod when user subscribes TMPE
// * enabled via content manager > mods
// * already enabled when game starts
// * hot-reload (can happen in-game, main menu, etc)
public void OnEnabled() {
    if (!HotReload) {
        LogEnvironmentDetails();
        PerformCompatibiltyChecks();
    }
}

// comment...
public void OnDisabled() {
    RemoveEventListeners();
    if (HotReload) {
        Patches.Remove();
        ModUI.Remove();
        //...
    }
}

Basically, we should be able to see at a glance wtf is going on without having to wade through gargantuan amounts of code.

The TrafficManagerMod.cs would then be like (rough mockup):

public void OnEnabled() {
    Lifecycle.OnEnabled();
}

public void OnDisabled() {
    Lifecycle.OnDisabled();
}
@originalfoo originalfoo added discussion Contains debate on certain topics technical Tasks that need to be performed in order to improve quality and maintainability code cleanup Refactor code, remove old code, improve maintainability labels Mar 2, 2020
@originalfoo
Copy link
Member Author

Thanks for fixing the typos! There are too many devs whose name starts with k lol.

@kianzarrin
Copy link
Collaborator

Related #730 #731

@originalfoo originalfoo modified the milestones: 11.1.2, 11.2 Mar 2, 2020
@originalfoo
Copy link
Member Author

Current progress (removed code comments for sake of brevity) - the stuff from IUserMod is done:

namespace TrafficManager {
    using CSUtil.Commons;
    using ICities;
    using TrafficManager.State;
    using TrafficManager.UI;

    public class Lifecycle { // will eventually be : ILifecycle

        private static Lifecycle instance_;

        public static Lifecycle Instance => instance_ ?? (instance_ = new Lifecycle());

        public void OnEnabled(bool hotLoad) {
            if (hotLoad) {
                Log.Info("HOT RELOAD");
            } else {
                Temp.LogEnvironmentDetails();
            }
        }

        public void OnLocaleChange(string locale) {
            Translation.HandleGameLocaleChange();
        }

        public void OnSettings(UIHelperBase helper) {
            Options.MakeSettings(helper);
        }

        public bool OnCompatibilityCheck() {
            return Temp.CheckCompatibility();
        }

        public void OnDisabled(bool hotUnload) {
            Log.Info("TM:PE disabled.");
        }

    }
}

Hopefully that clearly conveys what happens at those stages of lifecycle. Need to do some minor tidy up then will create draft PR.

The Temp class, as it's name suggests, is just temporary dumping ground for some bits of code (all of that will be replaced by new compatibility checker in #699).

The goal is that the Harmony/Lifecycle mod would call those "lifecycle event handlers", but for now I've got existing IUserMod calling them.

@originalfoo originalfoo modified the milestones: 11.2, 11.3 Mar 28, 2020
@kianzarrin kianzarrin self-assigned this Apr 8, 2021
@kianzarrin kianzarrin linked a pull request Apr 10, 2021 that will close this issue
@originalfoo originalfoo modified the milestones: Planned stuff, 11.6.0 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability discussion Contains debate on certain topics technical Tasks that need to be performed in order to improve quality and maintainability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants