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

Investigate registering listeners on the shell menu #297

Open
robojumper opened this issue Oct 12, 2018 · 16 comments
Open

Investigate registering listeners on the shell menu #297

robojumper opened this issue Oct 12, 2018 · 16 comments

Comments

@robojumper
Copy link
Member

Currently, X2EventListeners are registered in tactical, and in strategy. The fact that they don't work on the shell screen was a drawback we accepted whenever we decided to use an event hook instead of a DLC hook.

TLE moves some UI to the shell that may be relevant for event hooks (dynamic class icons, for example). We should investigate whether registering on the shell is wanted.

@GingerAvalanche
Copy link
Contributor

What are the downsides to using a new boolean RegisterInShell for the CHEventListenerTemplate (or even for X2EventListenerTemplate) and then registering event listeners with that boolean set to true, in OnInit of UIShell or UIFinalShell, and then removing them prior to unloading the shell level?

@robojumper
Copy link
Member Author

Update: This seems somewhat difficult, as there are lots of places on the shell menu where the history is reset, clearing all registered listeners. Probably better to wait for a real use case or problem before attempting this...

@Iridar
Copy link
Contributor

Iridar commented Feb 23, 2020

Well the Issue #783 would be a real use case, I even have the Event done and set up. I'm perfectly content with changing it to a DLC hook, though. I'm not sure if there's something that an Event would accomplish, but a DLC hook would not, in this specific case, since there's all event listeners would have to use ELD_Immediate to take advantage of it anyway.

@Xymanek
Copy link
Member

Xymanek commented Feb 24, 2020

Somewhat related: personally, I think we should register strategy listeners before calling CreateStrategyGameStart - the setup process may be invoking CHL extension events which are currently getting "lost"

@Musashi1584
Copy link
Contributor

I have a new "real use case" with events for the mcm actions in my mcmbuilder that should work in the shell too. @robojumper is this something you would consider to pick up again?

@robojumper
Copy link
Member Author

Conceptually, the shell is much closer to strategy than tactical. Are there any drawbacks to just registering strategy listeners on the shell?

@Xymanek
Copy link
Member

Xymanek commented May 28, 2020

Strategy listeners can expect things like HQPRES to exist, which would no longer be the case

@Musashi1584
Copy link
Contributor

Musashi1584 commented May 29, 2020

Just tested calling class'X2EventListenerTemplateManager'.static.RegisterStrategyListeners(); in a UISL on the shell enables strategy listeners to receive events.

@Musashi1584
Copy link
Contributor

Strategy listeners can expect things like HQPRES to exist, which would no longer be the case

wouldn't none checking that be in the error handling responsibility of the listener code?

@Xymanek
Copy link
Member

Xymanek commented May 30, 2020

Why would you check for existence of global singletons?

@Musashi1584
Copy link
Contributor

Why would you check for existence of global singletons?

Cause there are obviously situation where they are not there

@Xymanek
Copy link
Member

Xymanek commented May 30, 2020

Please state an example of HQPRES == none while in strategy

@Musashi1584
Copy link
Contributor

if(XComHeadquartersGame(class'Engine'.static.GetCurrentWorldInfo().Game) == None || `HQPRES == None) //Clean up log spam -BET

@Xymanek
Copy link
Member

Xymanek commented May 30, 2020

That is an example of a check. It might've been wrong, for old code, called too early into strategy setup or many other reasons.

What I'm asking is an example scenario which would cause HQPRESS == none for a "generic"/"normal" bRegisterInStrategy listener

@Musashi1584
Copy link
Contributor

Musashi1584 commented May 30, 2020

The point is you can also listen to events beside event templates which have no bIsStrategy so perse there is no guarantee when a listener is called. I see your point and I am not against a new bIsShell or whatever. Just saying making assumptions in code always having the possibility of error when code changes or the assumption is not true for an edge case.

@pledbrook
Copy link
Contributor

pledbrook commented Dec 22, 2020

So the two linked issues only need to have listeners registered in time for campaign initialisation, not necessarily as soon as the shell menu is displayed. I don't know if that would suit Musashi's use case, but it should certainly be pretty straightforward to implement.

I would also like to voice my support for having a new type of listener that isn't either strategy or tactical. I think there's a strong chance that registering existing strategy listeners before a campaign is initialised will break some mods. Unless we can guarantee that none the existing strategy events are fired at an earlier point than they currently are.

pledbrook added a commit to long-war-2/X2WOTCCommunityHighlander that referenced this issue Dec 23, 2020
This change adds an extra `RegisterInCampaignStart` boolean to CHL event
listener templates (on top of the existing `RegisterInTactical` and
`RegisterInStrategy` ones). Listeners registered for campaign start will
now receive events that are fired during campaign initialization, which
strategy listeners don't receive.

Resolves X2CommunityCore#297.
pledbrook added a commit to long-war-2/X2WOTCCommunityHighlander that referenced this issue Dec 23, 2020
This change adds an extra `RegisterInCampaignStart` boolean to CHL event
listener templates (on top of the existing `RegisterInTactical` and
`RegisterInStrategy` ones). Listeners registered for campaign start will
now receive events that are fired during campaign initialization, which
strategy listeners don't receive.

Resolves X2CommunityCore#297.
pledbrook added a commit to long-war-2/X2WOTCCommunityHighlander that referenced this issue Dec 23, 2020
This change adds an extra `RegisterInCampaignStart` boolean to CHL event
listener templates (on top of the existing `RegisterInTactical` and
`RegisterInStrategy` ones). Listeners registered for campaign start will
now receive events that are fired during campaign initialization, which
strategy listeners don't receive.

Resolves X2CommunityCore#297.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants