- User Since
- Jan 24 2017, 12:54 PM (116 w, 3 d)
Tue, Apr 16
concern fixed by rP20939
Mon, Apr 15
Cav got removed from ranges (which makes sense)
Some bot is complaining that this patch is rotting to long and needs a license year update
Renamings of spear => spearman and co is good imo (wasn't there some earlier revision where it was discussed?)
Sun, Apr 14
Comments By: elexis, Imarok
proofreading the gamesetup xml and global objects says the patch is complete (did not read the full file for any hidden tooltip in the code)
Thx for the patch
Fri, Apr 12
Reading it again, the actual question is how to treat special (the rest looks good)
Changes to trivial to not accept,
Whatever makes sense compared to the other palisade structures. e.g cost wise a rocks_curve is pretty similar to a rocks_long, right?
I assume agricultural products would be the most general term? Do we want that in game?
Sun, Apr 7
I get that ppl can choose whatever name they want, but who is named 4c905de7e2c9950b7d83273a8070b072 (second one in the german list)? I tried finding him/her in transifex but no luck.
I won't stop anyone for making/commiting a temporal fix, but the underlying problem is that we have two components (buidlingAI and unitAI) on the same unit trying to achieve the same thing (attack something). In the end that is the thing we need to fix (as in subunits)
Sat, Apr 6
Calling the traders out of scope of this patch
Checked the difference between the templates as they are loading in game (by printing them out), it resulted in a number of actual changes, giving them below, some of them are not a problem (or actually make sense), a few need to be addressed.
That must be a copy-pasta, text.delete.right doesn't have it, so completeness in that sense
changing vegetables to grain wouldn't work since f.e. the Chinese mod has rise fields instead. One could consider "crops" though.
reads correct, greps complete, unit demo works => accept
Fri, Apr 5
As elexis mentioned the color animation would in the vanilla game be dead code, thus that shouldn't be there. Ofcourse it could be useful later, so I would propose to implement the system for the size only, but keep it extendable so the color could easily be added when required.
Mar 18 2019
Grepping says these are all cases of seige, so correct and complete
Mar 17 2019
Reads correct, checked all formation templates => accept
I get that some languages use singular for every other prime number, so to speak. But isn't exactly that the reason why we have markForPluralTranslation, so a translation can choose for which values it takes whatever form. If that isn't the case in the current code, we might have found a bug in our code. Stating the same problem more concretely: how is this particular call different from all the other markForPluralTranslation calls? or are all of them just wrong? (there are some in buildrestriction, wonderVictory, CaptureTheRelic, triggerHelper and Treasure Island)
Code should stand on itself, not referring to a phabricator revision. But isn't markForPluralTranslation designed to cover this case? So how does this solve the issue?
Furthermore markForPlural expects 3 arguments, now it only has 2.
Is that lengthy comment really required? Seems obvious from the code (or it is me who has such a language as mother tongue)
Period complaint while at it
Candidate != must be moved, I guess those should be in the children then...
GarrisonHolder: change the Max attribute in the childs, put the rest in the parent
That is a good point indeed, so we should be looking at which properties we can move to the military parent. Territoy, Vision, GarrsionHolder and Sound seem to be the biggest candidates
Can I propose to make a template_structure_military_training (or find better name) template as a parent of barracks, stable and range? That would solve the duplication of this patch and D1790
Patch reads correct, unit demo test succeeded, grep gives translate hate=> accept
Mar 16 2019
Idea is ok, but patch is inclomplete: your grep doesn't seem to have found the skirmish buildings (the templates under skirmish/structures/)
Indeed inconsistent and misleading naming. Patch correct and complete, front doens't fall, also not in atlas
Game works, Atlas works changes make sense => accept
The option doesn't do anything outside the in-game so I suppose in-Game is the place...
One fairly trivial change required => accept
Something in the lioness changed, small rebase
Broke the mp while at it, fix incoming
@smiley: what would you think a "toFixed" would do on a vector? What we wanted to achieve is reducing the size of the command.txt for positioning units, thus dropping the precision after the 3rd decimal (as who cares about that?). To avoid duplication for any further such request a toFixed was added in the vector file. Regarding your example: if you use toFixed you expect a string, thus + gets a certain definition, so I don't see anything wrong....
The horizontal size indeed is computed in js, since that size can change when you click on another tab, however the vertical size is the same always so coded in the xml. What went wrong in the initial slide panel commit (so it's not you who broke things nani, I did) is that the xml value or the background and front panel got different value. This was then masked by the ontick, but when that was removed, the real bug came out. However notice that the bug only happens when the panel width is maximal: in a small window everything is just aligned (Probably the reason I didn't find it....).
Jan 19 2019
Jan 6 2019
well from a ownershipchange we can't see if it will be a rename, so when the new ent is brought up, we don't know it actually replaces another (one could change the entitylimits and allow two wonders, so an owners check wouldn't work), so the new timer will always be activated, which afterwards needs to be removed. Also from the destroy we don't know if it is a rename (one could very well create and destroy different wonders at the same turn), so we have to do it with the ent rename.
Jan 5 2019
grepped for completeness, agreeing on the change, translators will hate
(note to self: be careful with file move)
should be fixed now: rP22030