civfanatics / CQUI_Community-Edition

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

Fixed issue where Barbarian Banners did not clear when dispersed #276

Closed the-m4a closed 3 years ago

the-m4a commented 3 years ago

Finally figured out what was going on - calling Reload() when updating settings caused all of the OnImprovementAddedToMap() events to be called, which in turn would add a new instance of the Barbarian Banner over the top of the existing one. As such, when the camp was cleared, one of the banners would also clear, but because they all did not clear it looks like nothing happened.

I made one other minor fix and updated the CQUI version number that appears in the log, as that number actually helped me solve a different issue for someone on Reddit.

the-m4a commented 3 years ago

I think we get hurt by exactly how Firaxis allows the modding of these components to happen - we're especially hampered by the XML files. The "include" element in the XMLs help that a little bit, but by and large whenever there's changes to the controls we have to replace the entire XML. That hurts the Lua too, because often shared code would be dealing with a similar component. Typically the mods are one-to-component, with exceptions made when there's enough desire there for folks to use both at the same time (e.g. Civitas city states, Sukitract's Simple UI)...

I don't know if it's a matter of me not actively looking for areas where code share could occur, or if it's I looked in the past and there was enough examples of where it was blocked and couldn't happen where I just kind of didn't prioritize the idea. It's a good idea, just not sure how to really pull it off at this point.

For example - I like that CIVITAS City States mod. The extra kind of city states added is neat. I also have come to really like the inline quests idea that @Swarsele brought to us. Because of working to make CQUI and CIVITAS work together in the past, I found the CIVITAS publisher/owner/etc and offered to add the inline quests to Civitas. I sent a PR over that way last night after getting it to work - which did include a neat trick to allow the CQUI setting be used for the font size, if CQUI was also running. That said, because both CQUI and CIVITAS make changes to CityStates.xml, each adding different elements to the "CityStateRow" instance, there needed to be changes to the CIVITAS citystates.xml in order to facilitate this. There does exist a way to make the Lua stuff work - it's basically the same mechanism we use for most of our files - as long as both of the mods get the opportunity to load the lua file. That stuff that Firaxis does with the CityBannerManager.lua file - where it has include("CityBannerManager",true) at the bottom of the file - means anything named CityBannerManager*.lua will be "included".

Sorry that was a long way to say: it's intriguing, but I don't know if it's possible and if we have a reasonable amount that would be sharable...