Differential D268
Add "mul_round" op to template parsing to support multiplying with integer types. Authored by wraitii on Mar 27 2017, 5:23 PM.
Details
Sometimes when using "mul", the values come out decimal. This may happen in situation where the validation expects an integer result (as it was for cost before for example), leading us to change that to Decimal. This is a poor solution to the problem. I think it is preferable that the paramNode code is not aware of the validation, so I think the simplest solution is to allow rounding following an attribute. See code. Check out the code, test it, approve/disapprove the idea
Diff Detail
Event TimelineComment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/619/ for more details. Comment Actions No, go fix that one TODO in the relative template code if you need it for that. Also non-int costs are stupid. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/620/ for more details. Comment Actions 2 things: Comment Actions They are. You're not changing only time here, and sure something costing half a wood totally makes sense.
If you don't consider that whole component to be just that. Comment Actions Going to rephrase this revision to address #3818. @leper: I thought about this a bit and I don't think validation schemas should have an influence on templates. I would rather go with a user specified parameter, something like:
Would need minimal code change, would lead validation to still fail if you forget to round, meaning the template code has more control, and so on. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/1058/ for more details. Comment Actions
Yes, but the templates should conform to the schemas, else why even have schemas (I mean NoSQL dbs also work just fine (or at least nobody can tell if they don't)?
This allows specifying both round and ceil which seems like a bad idea from a usability perspective. Also why not just have eg imul in case we don't want to rely on the schema when parsing. (Would also make the CParamNode code nicer IMO.)
Comment Actions I'm ok with using imul but it doesn't allow choosing between flooring, rounding and ceiling which would be convenient imo. I would also like a
argument instead but I'd like a better name than round and couldn't think of one.
Comment Actions I'm not sure if it is really worth bothering with providing all of those, since if someone cared enough about the specific value they would provide it in the template, instead of making balancing that much easier.
rounding_mode, but that also sounds bad.
Comment Actions Use mul_round instead of 3 different attributes. The woman template is still just to show the code works. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/1100/ for more details.
Comment Actions What about Health? If we don't have fractional health, then attack damage should be rounded too. But if attack damage is rounded, the tooltips display a misleading value. Perhaps that could be solved with a ssmall GUI page that displays the actual damage dealt to some templates (basically that templatesanalyzer ingame). Comment Actions Possible values in templates and whatever components do with that are different things.
Comment Actions Build is green Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK! Checking XML files... http://jenkins-master:8080/job/phabricator/2189/ for more details. Comment Actions Executing section Default... Executing section Source... Executing section JS... http://jenkins-master:8080/job/phabricator_lint/645/ for more details. Comment Actions (imul sounds better than mul_round.) The other approach seems really hard. @leper had you some vague ideas when writing the TODO? Comment Actions I'd agree, but mul_round is clearer and I don't believe all of our modders would instantly understand imul. Comment Actions
Probably, else I wouldn't have written the TODO. Then again it has been 2 years, so dunno. Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/863/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||