Subsystems have a very nice feature, where in Initialize they can declare their dependency through calling InitializeDependency, and the system makes sure all subsystems are initialized in the correct order of dependencies.
But then when it comes to calling literally any other subsystem-wide notification (OnWorldBeginPlay, OnWorldComponentsUpdated, OnWorldEndPlay, etc.., these are just examples for World subsystems), this dependency order is discarded and the notification is sent in random order (it’s not entirely random, it depends on which order they are given in TMap<UClass*,…>, but it’s definetely not predictable, and often doesn’t correspond to initialization order).
Is there a good reason for this order of execution, other than that it was easier to implement this way (and slightly more memory efficient)?
From what I understand, the current best practice is to create a multicast delegate on the prerequisite system, and listen to it in my dependant system. This works for a single dependency, but when a system depends on several other systems, it would be much easier to just have them be notified earlier automatically.
[Attachment Removed]
Hello, the subsystem collection-wide function calls are indeed unordered.
“Is there a good reason for this order of execution, other than that it was easier to implement this way”
It was just straightforward to implement that way, and hasn’t been revisited to make it more sophisticated.
“From what I understand, the current best practice is to create a multicast delegate on the prerequisite system, and listen to it in my dependant system.”
That’s a good approach that doesn’t require engine changes.
With engine changes, you have a few more options. Most of the subsystem functions, including your world examples, are called via FSubsystemCollectionBase::ForEachSubsystem. That calls FSubsystemCollectionBase::FindAndPopulateSubsystemArrayInternal, which creates an array on demand from the TMap. With engine modifications, you can choose to sort the array inside that function:
- Subsystems’ dependencies aren’t cached, as I believe you’re aware of based on the memory efficiency comment. You could cache the dependencies by modifying the implementation of InitializeDependency() and somehow mapping the dependency to the subsystem requesting the dependency.
- Then, in FSubsystemCollectionBase::FindAndPopulateSubsystemArrayInternal, you can add some code to sort NewList based on dependencies. For example, by calling HeapSort on the array with a comparison function that checks dependencies. See FHierarchicalLODBuilder::FindMST() for an in-engine example.
So that’s an alternative you can consider. I’m curious to hear what you’ll decide on.
[Attachment Removed]
Hello! That’s a nice discovery. I verified your observation:
- Converting the TMap to TArray on-demand, like in ForEachSubsystem if the array doesn’t exist yet, has no predefined order
- If the array already exists earlier, during subsystem creation, each initialized subsystem is appended at the end
I don’t think we would merge in this change, because although it “fixes” the order of iterating USubsystem, it doesn’t change the order of other calls like ForEachSubsystemOfClass and ForEachSubsystemWithInterface. Then, one call would adhere to the initialization order, while the others wouldn’t. (One could apply the same trick as you did by pre-creating arrays for all classes, but that would take more memory.)
Currently, the FSubsystemCollectionBase ForEach functions don’t promise that they execute functions in an order that follows initialization dependencies. I think keeping that is better than inconsistent behavior for the different ForEach function types. An engine modification on your end makes sense, though.
[Attachment Removed]
That is great to hear! Apologies for misunderstanding your earlier implementation description. I will ask for opinions from the rest of the team to hear whether we would want to grab this change.
You can wait to prepare a CL until I know the team’s opinion. I want to be respectful of your time.
[Attachment Removed]
So far, the appetite within the team to change the general iteration order of subsystems (via ForEachSubsystemWithClass/Interface, which includes OnWorldBeginPlay) isn’t strong. Can you provide some context on how OnWorldBeginPlay must happen in a certain order for your subsystems and why that applies to other functions in your game as well? That will help us reason about whether this would be a general need for devs. 
Looking through past discussions about subsystem order, at some point there was a change in the “arbitrary” way that subsystem classes are discovered. It unveiled implicit subsystem dependencies, causing bugs in our own projects and licensee projects. They were fixable with proper InitializeDependency calls, but it goes to show that backwards compatibility would be affected if we changed execution order. Any change in behaviour would probably be behind a cvar.
[Attachment Removed]
Closing this. To summarize:
- I believe the change you made locally to have some ordering on subsystems is fine as local engine change
- The team doesn’t want to adopt it, so a PR would be wasted effort
Thank you for raising this and offering the PR!
[Attachment Removed]
First of all, my bad, we are not on 5.7 but actually on CL 47892199. However there are not significant changes regarding this issue.
I have found what I think is quite elegant solution. If I add USubsystem (which is a parent to all subsystems) to the SubsystemArrayMap before initializing any subsystems (at the start of Initialize), then it will be added to this list (AddAndInitializeValidatedSubsystem already does that) in the correct order (every subsystem will appear in this list after all of it’s dependencies).
Then I can simply replace both cases of iterating over SubsystemMap with iterating over SubsystemArrayMap[USubsystem::StaticClass()], ensuring that both ForEachSubsystem and any future SubsystemArrayMap entries always preserve this one “canonical” ordering of subsystems.
As a small bonus, the entry for USubsystem in SubsystemArrayMap would have been created anyway the first time ForEachSubsystem() is called (for example for World subsystems this happens immediatelly when dispatching OnWorldBeginPlay), so this doesn’t even increase the memory footprint.
Would you be interested in merging this change? I believe it is fully backwards compatible (it adds a guaranteed ordering to something that was “random” before) and the changes are minimal.
[Attachment Removed]
What I did was I also modified the iteration in FindAndPopulateSubsystemArray to use the same predefined order. Because of that, all arrays created by this method (which is internally used by all the ForEach variants) are also in the same order (even if they contain only a subset of subsystem). If a new subsystem is added later, it is correctly appended at the end of each of the SubsystemArrayMap entries, preserving the order further.
Basically, all entries of SubsystemArrayMap are now in the same order at all times.
[Attachment Removed]