Add extra popup and notification settings #353
Conversation
|
Can you just create a PR for #182? |
|
Sure thing! I'll try to get that done sometime today or tomorrow and get that submitted as a separate PR. |
|
As requested, that feature was split off into #356. I'll rebase this PR if and when that one gets merged. |
|
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. |
|
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
force-pushed
the
PopupsAndNotifications
branch
2 times, most recently
from
f4ef2a4 to
f7df325
Compare
JamieNyanchi
force-pushed
the
PopupsAndNotifications
branch
2 times, most recently
from
b7f6bcc to
f73c73b
Compare
JamieNyanchi
force-pushed
the
PopupsAndNotifications
branch
from
f73c73b to
bb7ec57
Compare
Adds the following two features: 1. Allows more automatic popups/visuals to be disabled. Added popup settings include ones for the tech/civic boosted, unit captured, natural wonder revealed, space race project completed, and rock band popups/visuals. 2. Allow popups/visuals to optionally show in multiplayer.
JamieNyanchi
force-pushed
the
PopupsAndNotifications
branch
from
bb7ec57 to
3d20eb4
Compare
|
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! |
|
This PR has too much of a new code, and the value added is really miniscule. |
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:
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.luafile 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!