search

civfanatics / CQUI_Community-Edition

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

Unhide Alliance tab for teammates #346

Open JamieNyanchi opened 1 year ago

JamieNyanchi commented 1 year ago

The purpose of this change is to unhide the Alliance tab in the DiplomacyActionView when playing on a team. The game normally hides the Alliance tab when you're on the same team as the leader you're currently viewing. This is confusing behavior though as you are still able to form alliances with other leaders on your team. The information behind the Alliance tab isn't meant to be a secret to the player, so it can be unhidden without negatively impacting any aspect of the game. The associated commit simply overrides the base game's AddIntelAlliance function and removes the team check at the beginning.

I believe this change falls within the scope of CQUI as having access to this information can help you more easily and quickly manage your alliances since you don't have to try to keep track of how many alliance points you have, for instance.

Infixo commented 1 year ago

How does the vanilla game UI work here? Does it show the info or not?

JamieNyanchi commented 1 year ago

In the vanilla game, if you open up the diplomacy action view with another player who is on the same team as you, it will hide the alliance tab. If the other player is not on the same team as you, it will show. So, as it is right now, CQUI is doing vanilla behavior. I updated the PR to better show the part of the overridden function I actually modified. I only commented out the the if statement at the beginning. The rest of the function is unchanged.

As for my justification for making this change, I just don't think it makes sense to hide this tab. Maybe at some point it was intended for teammates to not form alliances with each other, but in any case, they are able to form alliances just fine in the vanilla game. Unhiding this tab doesn't show any secret information either as the tab only shows the current status of your alliance (current alliance level and alliance points).

Infixo commented 1 year ago

I am not willing to change vanilla behavior especially in regards to multiplayer. We must be sure that this is actually needed and vanilla behavior is erroneous.

JamieNyanchi commented 1 year ago

Hmm, the best evidence I can provide that this may be erroneous is this post from the Civfanatics forums: https://forums.civfanatics.com/threads/1-0-0-290-reproducible-alliance-status-not-visible-in-multiplayer-teams-1-0-0-314-still-exists.643739/

If I understand correctly, user dshirk is a Firaxis employee who was also a lead producer for Civ 6. His response to this makes me think this was erroneous and it was supposed to be changed. However, it apparently never made it to the released version and I can't find why. In any case, his message seems to imply the current behavior is not intended.

To provide a bit of extra clarity just to be safe, this doesn't change whether or not you can make alliances. You've always been able to do that on teams (once you've gotten the related civic of course). This just changes the visibility of the tab that shows you the status of your alliance. This is visible with everyone who isn't on your team, so it just seems strange for it to not be visible here. You could even manually keep track of the progress yourself right now too, so it's not necessarily secret information.

All that said, I do understand the concern with making this change if it actually was intended to be the way it is. That's why I think the post on the Civfanatics forum is the best evidence that it isn't intended the way it is.

the-m4a commented 1 year ago

I agree with Jamie - this seems like a pretty straightforward fix that's not breaking game rules. I am fine with this change.

the-m4a commented 1 year ago

Approving, but will wait to merge pending comment from Infixo if desired to do so

JamieNyanchi commented 1 year ago

Ah, so that's the preferred coding system here after all! I honestly wasn't sure which was preferred. I'll keep that in mind for the future! That probably also means I need to fix the other PR I have open... Anyway, I have moved the code so it's only in the base file. If I'm not mistaken, this should be okay without expansion checks as this function is only called in Rise and Fall and Gathering Storm (and is identical in both cases). The local variable that is added is also only used by this function.

Infixo commented 1 year ago

Yes :) Just make sure it is called when necessary :) @the-m4a spent lots of time cleaning those files :)