Units should only be able to build structures from their owner's civ, and not from their identity civ
Reviewed By: elexis
Trac Tickets: #4870
Differential Revision: https://code.wildfiregames.com/D1065
Units should only be able to build structures from their owner's civ, and not…
Description
Details
Event TimelineComment Actions I guess reading the Vulcan output helps. I started writing a patch, but I can go on with my other atlas bug inside the atlas bug revealed by the removed rmgen bug if you happen to write a builder fix as well Comment Actions Didn't start writing a patch yet, could do though or continue reviewing some more patches at random Comment Actions Seems to be more like the problem that Vulcan says "successful build", while the tests fail Comment Actions This now prevents C&C style capturing (which allows one to build everything from the other faction). It also somewhat breaks that one cheat. Seems like a workaround for an AI issue. Comment Actions Those just state "Yeah, that shouldn't be done and now we'll enshrine that after not checking any previous discussion after less than a day of discussion.". Comment Actions No, what was done previously just had no sense. If we want to only allow to build structures from the Identity civ, then the unit template should just not have {civ} but the civ we want: It would be useless to have that {civ} replacement here,. The behaviour of the patch (replaced by the civ player, as is done in the ProductionQueue for units) is what has always been expected but was buggy. Comment Actions
Ah, I guess I was somewhat confused by the comment that wasn't fixed in rP16506 that still talks about the entity. Same for the schema here and in the production queue. Also having to specify the civ explicitly for C&C style capturing seems strange, given that having {civ} is because we want to make that easier. Though I'm not sure where a flag to switch from one behaviour to the other would even fit.
As is apparently broke if it doesn't get what it expects, that seems strange. Comment Actions
I do believe {civ} was added to make the identity civ replacement, but in general the way we've handled capturing meant that we needed to change that (which I've always found kind of annoying…). Maybe we should just replace it with {id_civ} and {owner_civ} and be done with it, to be honest. Comment Actions Gameplay wise, with the change we now have both possibilities: pers_house will allow the pers units to always build their own house, and {civ}_house to build the player one when captured. But i agree that there is a (minor) drawback when using templates inheritances. As wraitii, i would propose two replacements as the way to go, although a bit differently: Comment Actions I'd rather go with {owner_civ} and [ID_CIV] then, as that somewhat clarifies what we're looking at and vaguely ressembles a C++ #define in all-caps. In fact I'd be fine with #CIV but I feel like that'd be less understood. |