search

civfanatics / CQUI_Community-Edition

Civilization 6 mod - UI enhancements, reduce clicks and manage your empire faster!
MIT License
154 stars 28 forks source link

Add extra popup and notification settings #353

Open JamieNyanchi opened 1 year ago

JamieNyanchi commented 1 year ago

Quick Explanation

This pull request implements the following two features:

1. Allows more automatic popups/visuals to be disabled. Resolves #314. 2. Allow popups/visuals to optionally show in multiplayer.

I apologize for the large PR. These changes ended up being larger than I anticipated.

Detailed Explanation

Detailed explanation of changes --- **1.** Allows more automatic popups/visuals to be disabled. Settings for the following popups and popup movies were added: - Natural Wonder popup movies and audio - Space Race Project Completed popup movies - Tech/Civic Boosted popups - Unit Captured popup - Era Complete popups (The ones that say "The world enters the X Era") - Historical Moments popups - Natural Disaster popup movies - Rock Band popup movies - Secret Society Discovered/Joined popups - Hero Discovered/Expired popups All of these popups are enabled by default, as this is base game behavior. In other words, the default settings follow base game behavior. --- **2.** Allow popups/visuals to optionally show in multiplayer. I thought I saw a request for this somewhere, but I can't seem to find it anymore. In any case, this feature works properly without any impact on the multiplayer experience. The popups and popup movies will only show if the Multiplayer Popups option is enabled. By default, this is off to follow base game behavior. The `popupmanager.lua` file was modified to ensure that the multiplayer experience is not impacted by these changes. In the base game, these popups will lock the engine so nothing happens until the popup is closed. This is modified so that the engine will not lock in multiplayer so no other players will be impacted if one player has the popups enabled. Singleplayer is not affected by these changes. ---

Closing Comments and Notes

I have tested these changes both in Singleplayer and Multiplayer (both with and without the Heroes & Legends and Secret Societies game modes). My tests have mostly been on Gathering Storm, but I have done quick checks on the Base Game and Rise & Fall and the changes appear to work fine on those too.

As always, comments and reviews are greatly appreciated and I'm happy to make any changes necessary to move this forward!

Thanks in advance!

Infixo commented 1 year ago

Can you just create a PR for #182?

JamieNyanchi commented 1 year ago

Sure thing! I'll try to get that done sometime today or tomorrow and get that submitted as a separate PR.

JamieNyanchi commented 1 year ago

As requested, that feature was split off into #356. I'll rebase this PR if and when that one gets merged.

JamieNyanchi commented 1 year ago

Oh, you rebased it for me, thank you! I've tested the remaining two features a bit and I haven't come across any issues so far. I think the popup settings feature is all good to go. I haven't had the chance to test every case in the multiplayer popups one, but it seems fine so far. Like I said in the original post, it's a bit hacky since the popups and movies aren't intended to show in multiplayer, but it seems to work.

For reviewing, I think the most important part to look at in regards to the multiplayer popups feature is the PopupManager.lua file and how the other popup files interact with it. The movie popups that occur when building world wonders, discovering natural wonders, performing rock concerts, etc normally all lock the engine until the popup is done. I noticed this is done by calling the UI.ReferenceCurrentEvent() function, so I purposefully have it avoid that function in multiplayer. This seems to prevent the engine from locking and the game keeps moving even when the movie popups are occurring. I'm pointing this out to hopefully help you all with reviewing if you want this feature added. It's currently set to opt-in so nothing should be different if playing with default settings.

JamieNyanchi commented 1 year ago

Switching this to a draft so I can restructure the files to better match the code standards here as well as make a few more changes and test them.

JamieNyanchi commented 1 year ago

I took forever to get this done, but I believe this is finally in a good state to review! Please let me know your thoughts! If you have any questions, I'd be happy to answer!

Infixo commented 1 year ago

This PR has too much of a new code, and the value added is really miniscule.