"Creates a [tileFilter] improvement on a specific tile" doesn't work #6576
Comments
For those without discord, the linked message reads:Platform: Desktop OS: Windows 10 Message: Save Mods: Save Data: Show Saved Game |
|
Well, the code looks a bit unfinished. There's at least one expression evaluated without side effects or storing the results... Blame says Yair's work 14 months ago. Might have regressed in that time. Still, I would doubt at least one design decision: Which tile was selected is stored as improvementInProgress with turnsToImprovement==-1 and no direct association with the construction queue entry. So, even with the crash fixed, I'd expect weird behaviour when a Worker lands on such a tile, or when queueing several buildings that create a farm and then choosing the same tile for all of them and so on. Is this reproducible with the public+current version of Civilization 6 Mod or would one need a manual modification, as the OP seems to suggest? |
The test district is in the public ver. of Civ 6 mod, but is currently blacked out. So you need to just remove the /* and load the game. Or you could copy-paste this: |
Does not show in mod manager??? This one I assume - but it's not playable at all due to too many modcheck failures (from newgame - loading above game unsurprisingly crashes in TechManager.updateEra). That can't be 4.0.6 and the original Civilization 6 Mod. Meaning I'm no longer interested, amigo 🇨🇴 |
|
No, This one bruh |
|
Then you should have said so - besides, the name differs from what the linked message says, so that's another hidden manipulation to cunfose oss |
The savegame up above is not compatible with that mod, even after patching around the baseRuleset name -> Nation Brussels not found. Even after hacking all the city states to the different ones in your mod it fails - different natural wonders 🤬 . Have any better save to easily repro? By the way - my working notes currently look like this: UniqueType.CreatesOneImprovement support
|
Ah, I had fixed that as well in a local branch. I'll wait for your PR on this and work my thing around that |
Oh, right, now the mod is different... let me get you another save with it's own mod.
Imo the improvement should be instantly placed as well
So, simple option: destroy the improvement if city is razed; |
|
I said "my notes" so the question marks were addressed to me not you, all that was just to show it's far more complex to finish that feature than just fixing some mistake. I'm 90% sure the code won't handle buying properly, but that's easier seen in the debugger than by code inspection. Which is no fun as the game slows to a crawl - 10s per click - with that mod on a small map, probably #6468 again. And your second hint - nope there is no improvement to destroy, but leaving the "marker"(s) would be an unrecoverable impediment, thus extra code is still needed to remove them. Same as with pillage and nuke, really, those would have destroyed the "marker" without cleaning the queue so a finished building would not place the imp. As I said, it's abusing turnsToImprovement = -1 as marker. I'm gradually becoming more attracted to changing the marker to a fresh field - but I already centralized that so it's just two places in code (two because: respect existing code regions). ... but gracias for paying attention! |
|
Ok, here is another save, this is just with a simple specific mod: In order:
**Thanks Xlenstra for the edits ;) |
|
Platform: Desktop OS: Windows 10 Message: Save Mods: Save Data: Show Saved Game |
Save File |
Come on. Ooooold and boring and mouldy and buried in dust. Plain to see why here 😜 - yes go on, read that line, it tries to add an improvement name to the city construction queue - so just name your building and its improvement the same! Problem solved! 🙄 😆 |
|
Progress, see checklist above. Now I'm out of ideas - or, to be precise, there were ideas to improve the code in my nightmares, and I forgot to jot them down. That means testing. @archduque-pancake , I declare you testing manager. Please install Android Studio, and go ahead check out my branch and test. 😉 Seriously, is that within your scope or would you like to be able to test with a jar or apk? My tests were on a fresh game. Testing To-Do
Some of these may be quite hard to set up a test situation for, I know. Do your best. ALL others daring to help are of course welcome to try that branch out. I'm done with this for the moment, far too costly already, and may not react to questions for a day or two, but this won't be forgotten. |
|
Alrighty, I can do it with Android Studio, but if I could get the .jar it could speed up the testing, it would help me, but don't worry if you don't want to provide it. Anyways, thank you so much for your work on beforehand, testing will be started. |
here, for a limited time. Why and when has it become so bloated? |
Testing Progress
|
|
Testing results: All marked above are successful. Bugs:
|
By manual choice or by automatics? How could I repro? Should only be possible if the
Well of course that would only be possible if there's something to pillage??? The "under construction" "marker" doesn't count as such, but it would be incompatible with what pillaging usually does. By the way, I have no idea what the current status on pillaging roads is... And nukes don't "pseudo-pillage' all possible tiles, there's the random factor before that. When a nuke treats a non-improved tile there can't be a conflict with the "under construction" "marker", so that should be absolutely unchanged by this branch. There's no other options - either the marker protects from pillage+nuke, or pillage+nuke can remove an improvement (removing marker too) while also unqueueing the building, or additional data in save games & memory. I chose option two... The important part - either the building in the queue and the "under construction" "marker" are both still there or both gone. Unsure how to test that myself - maybe hotseat multiplayer? I noticed the improvement may significantly reduce the tile yield, and still that tile would be auto-chosen - if it's the only feasible one. Upstream, the AI has no way to know the actual "value" (as in AI measure of benefit) of the building, as the "value" of the improvement can't be taken into account, and even more extra code for that - sorry, I'm too lazy. I did the bare minimum to automation (I believe) to avoid conflicts, AI is not really my thing. |
Respect! I couldn't have done it in that time (well, I also had RL taking precedence for a while). Muchas Gracias. |
When I try - those have a red circle, and clicking that just stops the "ask for tile" thing, the building stays unqueued/unbought. As I envisioned the UI should behave (or would you rather it continue the process and ignore the red-tile-click?). If you get green for a mountain - a mod thing? |
|
... but I noticed a missing updateTileGroups call right after manually placing the target marker ... How did that happen? I fought hard earlier to make the mark visible when supposed to... |
Both, I could do it, and AI as well. I was using an Academy as the improvement.
Both gone when there is something to pillage.
I do it by setting two nations as humans, no need for multiplayer. |
|
... I think I should tweak the color when tiles are technically allowed but choosing them would be a disadvantage. I have yellow if it would stop a Worker construction in progress, but not it it would kill a valuable Improvement. (My Civ6 mod copy doesn't mark Districts as Irremovable so that could be a big difference when that "Test District" places a Farm on it). I wouldn't really like to code an AI value comparison to control colors showing desirability, however. Yellow too if there is an improvement of any kind (exl roads) there....
Ah, that is maybe normally guarded against being plonked onto mountains only by the unit not being able to enter??? Valuable hint. -> Yes, TileInfo.L529 |
Yeah, they are a bit hard to spot.
Yes, altough now that I think about it... it doesn't really matter, as the unique will only be only used in mods, so the modder should add "cannot be placed in []" if he doesn't want that to happen. |
Happy to hear. But - needs dox. Where would that go...? |
|
In the improvement json of course. For the case of Civ6 mod, districts are new so no problem there. |
Still? I upped the alpha a bit, and thought it a big improvement. Don't want to hide everything on that tile. Then again, you could gain insight into which tile might be best by looking before triggering the please choose tile.
No, the modder using the Unique themed here should find such special considerations in the modding wiki, and the problem is - auto generated file, no provision for extra text. Dox to help the modder find your solution, not dox left by the modder as to why his mod changes the Academy this way... |
Yeah, personally it's not a big deal, as it's mostly just on grassland tiles. And, the most important dots are the red and yellow ones, which are easy to see. So no need to change it if it was by me, I said it because other players might not see them, idk.
I don't think it would be that hard to realise a fix for [my improvement] being able to be placed in a tile I don't want. |
|
What I said earleir "a missing updateTileGroups call" was wrong - that highlight I was missing only shows when the queue entry is selected, as intended. So, that leaves us with only cosmetic changes so far? I can update the jar easier than the PR/online branch as I had to merge another branch to even be able to fluidly test (which is why it shrank by 30%), and I don't want to mix. That jar changes the "please select tile" highlights only. Sources later. |
|
It just came to me this is intended to replace the District Builder Units in Civ6 mods -> But no matter how, they either need protection or some (nonexisting) feature to refund the ability to grow to the allotted number of districts - if the builder is killed, the improvement nuked away (which can either place the "repair" state or destroy outright depending on uniques, and a repair state can be removed), - once you're one short you stay one short.... Not scope of that PR, but needs thought. |
Trying to use the unique of "Creates a [tileFilter] improvement on a specific tile", and the game reads it, but upon clicking one of the green circles the game crashes.
To Reproduce
"Creates a [Farm] improvement on a specific tile"Crash Log
(Unciv discord server, bugs channel)
https://discord.com/channels/586194543280390151/588357826758574106/966057960067432468
Expected behavior
Game shouldn't crash obviously, and it should then place the improvement on the selected tile when the building finishes.
The text was updated successfully, but these errors were encountered: