- User Since
- Jan 24 2017, 12:54 PM (190 w, 6 d)
Then there is the problem of translating. For instance, whilst ‘sword’ can refer to both single- and double-edged blades in English, this is not true for all languages. In Chinese, jiàn is the word for double-edged swords and dāo the term for single-edged swords (and knives). Thus in those languages “sword” would be wrong in about half of the cases.
This is actually easy to fix, we can add a context tag in the template, which will be extracted already. So I just added the code to pipe the context through and use it for translations. In example: one can put <AttackName context="Double-edged">Sword</AttackName> in the template. On this specific topic: I also added context for all sword wielding units.
Back to attackName
add context support
add context for swords
improve some names
Digging back to D555, this code must be ccmpSelectable copy-pasta
Sat, Sep 19
Tue, Sep 15
Mon, Sep 14
While reviewing noticed that an upgrade without a cost will add a weird newline in the tooltip. However this is a separate bug, patch incomming.
Sun, Sep 13
Weapon => WeaponName
Or make it useful by using sim-information in the tooltip? Although I guess PetraAI would break if one changes classes in-game.
Note too that the current implementation is broken: from the template we store it under visibleIdentityClasses. Hence the patch letting classes be modifyable, or the patch wanting classes in the simstate, should properly implement it.
Fri, Sep 11
Move the check to a loop below. This should help performance since most entities returned by the rangemanager will be in range. Also nukes a loop
Wed, Sep 9
Technologies were not very potent and hardly ever researched.
Tue, Sep 8
The only reason I can see (now) to keep the per player storage is when ordering a unit to gather somewhere, where also enemies are gathering, the unit maybe should kill the enemies before gathering. However this can be considered player responsibility too, sine he should be able too see the enemies in this case. If he can't see them (very large resourcesupply ent or so), it is fine this way. So probably behavior we don't need want.
All cases of CMessageMotionUpdate::OBSTRUCTED got the treatment
Both issues seem fixed.
Mon, Sep 7
Considering whether returning to many entities can cause a problem. These are the affected components:
BuildingAI: arrows can fire out of range now. Probably can get away with checking the proper range in the buildingAI (slightly bad that we compute the range twice, with increasing precision)
Auras: probably who cares in this case
buildrestiction: ranges got a bit modified => no problem, since the ranges are large compared to the obstructions, small nerf for fortresses and some other structures (they need to be placed further apart)
AlertRaiser: ranges modified => same story as buildrestrictions (small obstruction, large range).
unitAI => nothing really changes, we just respond to more entities, but we still move in range
Attacking/delayedDamaged => fixed by D2963
Trigger: Changed as they should.
stylistic issues can be fixed while committing
Sun, Sep 6
This patch allows buildings to be build too close to other buildings, outside territory and everything else that buildRestrictions forbids. We should somehow reuse that code here too.
Don't feel this is the correct way of approaching the issue. I rather have the buttons enabled for the right click.
Noted by elexis
Shouldn't we do something similar for all attackRange queries in the ai?
Make sure you consider rP23994 when committing
Use Cstr, nuke boost
Sat, Sep 5
They were used in the tests...
It is not as simple as that: the strings that need to be translated need to be extracted from the scripts, so "just setting a translate call" won't do anything, since the english strings won't be uploaded to transifex.
There are not that many possibilities (Capture, Melee, Ranged). Or just take and translate the node itself?
After D368 the nodes are arbitrary, so many more possibilities. Translating nodes themselves isn't possible either, since they can't be extracted. Moreover, nodes can't contain spaces, while names can (see my Machine gun creation).
Introducing a new node means changing a lot of templates, and more importantly, the names are arbitrary.
Changing templates needs to happen anyways and names will become arbitrary too. This patch is exactly designed for changing all the templates, such that we don't need to do it when changing the code.
AttackName => Weapon
Use name of the weapons everywhere, no more projectile names (k, javelin, but that is a weapon too)
Improve elephant and animal names
Fri, Sep 4
For some reason formation movement is broken after this commit: formationmembers get stuck at obstruction when colliding with them on a path. Reproducible by ordering a (large enough) formation to walk to the other side of a tree (or any obstruction really).
Looks correct from the code, individuals do the same too.
Checking all pushOrderFront calls in the unitIA shows this patch is complete
Note that some tooltips are wrapped to the next line (towers, siege tower), but it is not introduced, nor made worse by this patch
Note that internationalization is as broken as before (rP22754)
Thu, Sep 3
Instead of nuking the tech, I think there are some other option that should be considered (also combinations):
- Buff phase requirement to town/city
- Buff costs (50 food one can do before the first animal, higher amounts maybe not)
- Split in several parenting techs
A proposal: indent splash and statuseffects under the attacktype
Seems nice overall indeed. I do would like the splash issue to be fixed before merging.
Wed, Sep 2
Mon, Aug 31
Code looks complete and works as advertised.
D1015 should be abandoned after this patch: bug has gone away and unitmotion doesn't treat moving targets anymore
Fri, Aug 28
Absence of animation doesn't block merging, absence of ticket does ;)
Currently yes, but why disallow it for any future mod?
grepped for completion that all HandleAttackEffects got a treatment (should have done that earlier)
Thu, Aug 27
Don't understand the change from decimals to integers, sounds like a regression to me. If all known mods use integers fine, but why disable any future mod wanting decimals? Also what about techs/auras wanting a +x% on armour?
Wed, Aug 26
Code desperately asking for an animation, needs ticket or so.
Open question: do we need to put this in the unit viewer/tooltip? Maybe not since the tooltip will say it quickly.
Pre rP23548, the ordering was by phase, common agreement there broke that and hence better group buildings by function then.
females and slaves need the same treatment.
New people should be credited and heads should also be in the list, however, heads should also be credited as such, even former heads.
Tue, Aug 25
We should run
svn propset svn:eol-style native $(find . -type f -name "*.json") svn propset svn:mime-type text/json $(find . -type f -name "*.json")
from time to time in the repo, or let some bot do it
Good use of those range functions imo.
Code looks good.
Mon, Aug 24
Here is what happens:
- Note that first the closing animation of the gate has to be shown once, otherwise the bug does not occur. This (perfectly normal and wanted!!) closing animation sets unitAnimation (via cmpVisual) to play the animation once. Now when a unit is ejected, we go over UpdateGarrisonFlag in the garrisonHolder, which will reload the animation (this is wanted for the garrison flag ofc, it needs to wave in the wind). But for gates, unitAnimation only knows about the closing anim, and will play that again.
Seems like vulcan doesn't like removing files or so, tests work here
Jul 30 2020
Vulcan found some stuff in Resistance.js
Jul 27 2020
Looks correct, greps complete