Gathering Storm and 1.0.0.290

Azurency commented 5 years ago

Great, the new Gathering Storm expansion is out ! That's very exciting and we're going to update CQUI to continue to work with the new expansion.

👷‍♀️ About patching CQUI

Yes, we're aware of the issues. It's almost always resolved by merging the patch changes (and there're some) into the mod files, but often require us to adapt the mod changes to new features (like the all new default building queue), naming, functions, code removed/moved, etc. That's why it may take some time (I mean more than a day, waaay more), if the patch is big or if there're some breaking changes.

🔨 How we'll proceed

We'll try to (roughly) follow these steps, maybe not in that exact order :

[x] Make the UI files diff of the base game patch and have a quick look at them
[x] Make a list of which files in the mod need to be looked at
[x] Create a new branch : expansion2
[x] Patch the changes for CQUI (the hard work), maybe we'll go feature by feature this time (more details to come)
[x] Test the changes

💪 Helping cqui

It will take me a lot of time for me to update the files, review issues, test the pull requests and answer everybody. It's because I'm busy with work at the moment, I try to find as much time as I can to dedicate to this mod but sometime it's just not enough.

If you're a developer that know how to use git and GitHub (the PRs), you can help this mod ! I'll soon put a list of files that need to be changed, it's "just" merging the changes introduced by the update to the cqui files (and being careful not to remove/change any cqui feature along the way) by reviewing the diffs. But it may require some adaptations too so I'll flag what should be easy or not.

I want to help but I'm not a developer if you have time to spare to help cqui, we still need people to answers on GitHub, Steam and discord, moderate and label the issues on GitHub, ... You can join the discord and post a message there.

TL;DR

Do not create new issue for the new Gathering Storm patch
Post your comments/issues here (even though NOTHING is supposed to work right now, so it's useless to post issues here... 🤷‍♂️)
We'll update CQUI I think feature by feature, releasing playable versions along the way
We're working on it 😉

Let's get to work then 🤓 🎉

Azurency commented 5 years ago

Here are the diffs for the base game : https://github.com/Azurency/Civ6-UIFiles/commit/6a4521ce6f9211ee337b12882e5706c30771465a, and for expansion2 (over expansion1) https://github.com/Azurency/Civ6-UIFiles/commit/277a1a7d756608a084a6a037a2b1289129512ef8. It's quite a very big expansion 😉

Azurency commented 5 years ago

The list of files that need changes :

CQUI Changes :

[x] actionpanel.lua, some Cloud and Era related changes - some work
[x] additions/citypanelculture.lua, few changes regarding an EspionageView - should be easy
[x] choosers/civicschooser.lua,
[x] choosers/researchchooser.lua,
[x] citysupport.lua, some function moving around from civ6common, should be easy
[x] civ6common.lua,
[x] civilopedia/civilopediasupport.lua, base game now remember the last page opened, so no need to keep this file, removed from CQUI
[x] diplomacyactionview.lua, some additions, should be easy
[x] diplomacyactionview.xml, few line added, easy
[x] diplomacyribbon.lua, some changes that will probably collide with CQUI modification, require some work
[x] diplomacyribbon.xml, few changes, easy
[x] expansion1/citybanners/citybannermanager.lua,
[x] expansion1/citybanners/citybannermanager.xml,
[x] ingame.lua, some lens and popup changes, should be easy (Don't know if this file is still relevant to keep in CQUI, the only method added by CQUI is never used)
[x] instances/leadericon.lua, require some adaptation to fit the CQUI modifications, require some work
[x] instances/leadericon.xml, require some work
[x] launchbar.lua, some additions, should be easy
[x] launchbar.xml, three lines, easy
[x] mappinmanager.lua,
[x] panels/citypanel.lua,
[x] panels/citypanel.xml,
[x] panels/citypaneloverview.lua,
[x] panels/citypaneloverview.xml,
[x] panels/notificationpanel.lua, some changes, require some work
[x] panels/productionpanel.lua,
[x] panels/unitpanel.lua, lots of changes, require work
[x] panels/unitpanel.xml, some changes, should be easy
[x] partialscreenhooks.lua, few changes, should be easy
[x] partialscreenhooks.xml, 5 lines changed, easy
[x] partialscreens/citystates.lua,
[x] partialscreens/citystates.xml,
[x] popups/boostunlockedpopup.lua,
[ ] popups/greatpeoplepopup.lua,
[ ] popups/greatpeoplepopup.xml,
[x] popups/mappinlistpanel.xml,
[x] popups/techciviccompletedpopup.lua,
[x] popups/wonderbuiltpopup.lua,
[x] productionpanel.xml,
[x] replacements/citypaneloverview_expansion1.lua,
[x] replacements/citystates.xml,
[x] replacements/citysupport.lua,
[x] replacements/diplomacyactionview_expansion1.lua,
[x] replacements/worldtracker_expansion1.lua,
[x] screens/civicstree.lua,
[x] screens/governmentscreen.lua,
[x] screens/governmentscreen.xml,
[x] screens/techtree.lua,
[x] strategicview_mapplacement.lua,
[x] supportfunctions.lua,
[x] techandcivicsupport.lua,
[x] tooltips/plottooltip.lua,
[x] toppanel.lua,
[x] toppanel.xml,
[x] unitflagmanager.lua,
[x] unitflagmanager.xml,
[x] worldinput.lua,
[x] worldtracker.lua,
[x] worldtracker.xml,
[x] worldview/citybannermanager.lua,
[x] worldview/citybannermanager.xml,
[x] worldview/plotinfo.lua,
[x] worldview/plotinfo.xml,
[x] worldviewiconsmanager.lua

Integrations changes

BES :

[x] choosers/espionagechooser.lua,
[x] choosers/espionagechooser.xml,
[x] espionagesupport.lua,
[x] partialscreens/espionageoverview.lua,
[x] partialscreens/espionageoverview.xml,
[x] replacements/espionagechooser_expansion1.lua,
[x] replacements/espionageoverview_expansion1.lua,

BTS :

[x] tradeoverview.lua,
[x] tradeoverview.xml,
[x] choosers/tradeoriginchooser.lua,
[x] choosers/traderoutechooser.lua,
[x] choosers/traderoutechooser.xml,

IDS :

[ ] diplomacydealview.lua,
[ ] diplomacydealview.xml,

ML :

[x] panels/modallenspanel.lua,
[x] panels/modallenspanel.xml,
[x] minimappanel.lua,
[x] minimappanel.xml,

URS :

[ ] replacements/reportscreen.xml,
[ ] replacements/reportscreen_expansion1.lua,

this list may not contain all file, as there're also new expansion2 file that will need to be created and modified expansion1 files that will need to be reviewed.

therealcrow999 commented 5 years ago

Don't forget to fix the World Tracker bug: https://github.com/Azurency/CQUI_Community-Edition/issues/14

You guys said you know what the issue is. Thanks.

ihendriks commented 5 years ago

Don't forget to fix the World Tracker bug:

14

You guys said you know what the issue is. Thanks.

I don't really see how this is relevant here.

joelekstrom commented 5 years ago

Hi! I wanted to try my hand at helping here. There are loads of diffs that are simply indentation/alignment changes - how do you propose managing those? Should we go with the indentation from the expansion2-files? They don't match the coding style of this repo. One approach might be to run it through some autoformatter, but that would require that the current state of this repo is also run through the same one first.

Perhaps the simplest thing (just converting to 2 space indentation) removes most of the diffs, but I also see quite a few diffs on alignment (for :types) etc.

And another question - how is one supposed to think about the two changesets? Does this mod keep compatibility branches for the base game and Rise & Fall? Shouldn't it be enough on the expansion2-branch to apply all files from Azurency/Civ6-UIFiles@277a1a7? I'm a bit confused here in what order one needs to check the both changesets.

I'd be very grateful if someone could point me in the right directions here!

majogl commented 5 years ago

Hi. Well good luck with all that. While doing this, could you please make sure if you check out this issue? It is from the old version, so not sure if it persists. It is in one of the submods, Better Trade Screen and is caused by bad interaction between it and YnAMP. Whenever on a larger YnAMP map with CQUI enabled, the trade screen would bug out. The issue contains a solution, so if it is still present now, just incorporate the solution. https://github.com/Azurency/CQUI_Community-Edition/issues/373 Thanks

bestekov commented 5 years ago

@accatyyc raised a good point about whitespace difference:

Hi! I wanted to try my hand at helping here. There are loads of diffs that are simply indentation/alignment changes - how do you propose managing those? Should we go with the indentation from the expansion2-files? They don't match the coding style of this repo. One approach might be to run it through some autoformatter, but that would require that the current state of this repo is also run through the same one first.

Was there any consensus on this? I have a dev background (and I did score display feature for CQUI a while back). I'm interested in pitching in and just wondering what the best way to start and not duplicate work is?

Azurency commented 5 years ago

Hi! I wanted to try my hand at helping here. There are loads of diffs that are simply indentation/alignment changes - how do you propose managing those? Should we go with the indentation from the expansion2-files? They don't match the coding style of this repo. One approach might be to run it through some autoformatter, but that would require that the current state of this repo is also run through the same one first.

Perhaps the simplest thing (just converting to 2 space indentation) removes most of the diffs, but I also see quite a few diffs on alignment (for :types) etc.

And another question - how is one supposed to think about the two changesets? Does this mod keep compatibility branches for the base game and Rise & Fall? Shouldn't it be enough on the expansion2-branch to apply all files from Azurency/Civ6-UIFiles@277a1a7? I'm a bit confused here in what order one needs to check the both changesets.

I'd be very grateful if someone could point me in the right directions here!

The indentation Well, It was decided a while back to switch to 2 spaces indentation (I was not involved in the project yet at that time) and since then, to avoid having unreadable diffs and keep track of what CQUI is changing in the files, we'll keep this 2 spaces indentation everywhere.

The compatibility Soooooo, as you can see if you look at how it works in the .modinfo, the mod contains files specific for each expansion, that's why there's a folder Assets/Expansion1 that contains the changes for the expansion1 which contains files that replaces files from the base game (or add new files). We'll soon create an Assets/Expansion2 folder. So with this expansion2, we have to change : the base game files (Azurency/Civ6-UIFiles@6a4521c), the expansion1 files (Azurency/Civ6-UIFiles@efd507f49a035904eb155b67535943620ffb4451) and look at the new expansion2 files that need to be modified by CQUI (Azurency/Civ6-UIFiles@277a1a7).

joelekstrom commented 5 years ago

Thanks! Right, I'm not 100% sure I follow, but I started a PR and will keep merging, just want to make sure I'm on the right track first. I'll try to read your post a few more times and check out how all the files are set up tonight. But please let me know if the initial commit was what you expect! (I have no experience with Civ VI-modding, so it's all new to me. I have spent the better part of my life merging code though 😉)

ihendriks commented 5 years ago

@accatyyc raised a good point about whitespace difference:

Hi! I wanted to try my hand at helping here. There are loads of diffs that are simply indentation/alignment changes - how do you propose managing those? Should we go with the indentation from the expansion2-files? They don't match the coding style of this repo. One approach might be to run it through some autoformatter, but that would require that the current state of this repo is also run through the same one first.

Was there any consensus on this? I have a dev background (and I did score display feature for CQUI a while back). I'm interested in pitching in and just wondering what the best way to start and not duplicate work is?

Just run with the double space indentation of this repo. The repo comes with an .editorconfig file, which can help with that. You probably have some indentation errors if you autoformat it, but those aren't too hard to spot usually.

With regards to duplicate work: The list in here is maintained pretty accurately as far as I can see, and if you are afraid of duplicating work you can also just comment on this issue saying what you're working on, that way someone else probably won't pick it up. I don't think there's that many people doing the diff work anyway.

Thanatos8088 commented 5 years ago

I have little help to offer the cause, but I don't see you guys getting told often enough how valuable and appreciated the work you do is to the end-user masses. I'll be sitting quietly over here waiting till you say "ready-ish", and gladly thank you again when that time comes.

Krassikn commented 5 years ago

I am a huge Civ fan but I want play the Gathering storm until your mod is finished. The game is unplayable without the mod. Many thanks for your great efforts and looking forward on finishing the mod.

ronimb commented 5 years ago

Playing without this mod is definitely a much more cumbersome, less refined experience. Can't wait for you guys to finish up, keep up the good work!

Azurency commented 5 years ago

Update : You can start to play with the expansion2 branch version. It's only missing :

the improved Great People popup
the Improved Deal Screen
the Updated Report Screen.

_(zip file of the latest version : https://github.com/Azurency/CQUI_Community-Edition/archive/expansion2.zip)_

thesatch commented 5 years ago

What a great job !!! Thank you so much !

AdolfEgzekutor commented 5 years ago

I found a hot-seat bug - when wonder is finished, game stuck on wonder animation/screen.

LyricL-Gitster commented 5 years ago

Nice to see others playing hot seat! My wife and I play that way but I'm sorry to say I haven't been able to help with the project yet...

BlackSmokeDMax commented 5 years ago

Not sure if I should be putting requests in with this not being fully released and all, but figured at worst the answer is no and it could be considered for the future.

Anyway, I'd love to see you include both the current version of the leader ribbon (where you can see score, tech, and military power) and when hovering over a leader show Aristos's new version ( https://steamcommunity.com/sharedfiles/filedetails/?id=1360462633 ) as a tooltip. Best of both worlds!

LyricL-Gitster commented 5 years ago

This is the place for discussion of progress toward releasing current features with the new update. It definitely sounds like you're requesting a new feature.

BlackSmokeDMax commented 5 years ago

Yes, I was definitely requesting a new feature. Just wasn't sure if it were something that could/would be added if it may not be easier to do it now rather than later.

bestekov commented 5 years ago

@BlackSmokeDMax, that would be a great feature. Recommend you open a separate issue requesting it so it can be worked/evaluated on its own. This issue is specifically for GS compatibility.

BlackSmokeDMax commented 5 years ago

Yep, I'll copy/paste that out to a new thread now.

trafenrest commented 5 years ago

I cannot research the future civic multiple times. After I researched it once, I can't select it any more. I have to end the turn by pressing shift+enter. When I disable CQUI, I do not have this problem. (This was a bug in the vanilla version some years ago.)

Not sure if the same thing happens for future tech...

wayneb64 commented 5 years ago

Could there be a bug with CQUI preventing canals from working properly? The UI is saying I only have one hex in my entire empire that is allowed to have a canal.

EDIT: Nevermind, seems like I don't fully understand the rules for canal building.

camk16 commented 5 years ago

Hey. Thanks to everyone who contributes to this mod! It seems to have stalled a bit, though- since the latest update a few weeks ago. The mod appears so close to being game-ready, so I'm curious why the push to get it there has suddenly stopped, so near its end.

ihendriks commented 5 years ago

Hey. Thanks to everyone who contributes to this mod! It seems to have stalled a bit, though- since the latest update a few weeks ago. The mod appears so close to being game-ready, so I'm curious why the push to get it there has suddenly stopped, so near its end.

Time constraints, mostly. I don't have much time to work on this at the moment, and I'm guessing @Azurency is having the same issue.

Azurency commented 5 years ago

I cannot research the future civic multiple times. After I researched it once, I can't select it any more. I have to end the turn by pressing shift+enter. When I disable CQUI, I do not have this problem. (This was a bug in the vanilla version some years ago.)

Not sure if the same thing happens for future tech...

should be fixed now

Azurency commented 5 years ago

I'll close this issue, the progress of the diplomacy deal view and the great people popup (the only things missing) can be tracked here : https://github.com/Azurency/CQUI_Community-Edition/issues/397

Azurency / CQUI_Community-Edition