Page MenuHomeWildfire Games

wraitii (Lancelot)
Animal

Projects

User Details

User Since
Dec 21 2016, 1:38 PM (122 w, 1 d)

Recent Activity

Yesterday

wraitii added inline comments to D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).
Thu, Apr 25, 3:11 PM
wraitii added a comment to D1828: Health.js cleanup: add tests, add an "IsInjured" function, use GetHitpoints everywhere.
In D1828#76156, @elexis wrote:

Seems unlikely that GetHitpoints returns something other than this.hitpoints, especially inside the same component.
I know the JIT can optimize, but I don't know if it can achieve this specific thing.
It's easy to change this.hitpoints if we ever need some logic change.
If we change it to use the function, then we should have a reason to (i.e. construct a realistic example where the logic would be different from return this.hitpoints).

Thu, Apr 25, 3:07 PM
wraitii added inline comments to D1839: Fix hotkey event sync with hotkey state..
Thu, Apr 25, 1:53 PM
wraitii added a comment to D1828: Health.js cleanup: add tests, add an "IsInjured" function, use GetHitpoints everywhere.
In D1828#76107, @elexis wrote:

But why replace this.hitpoints with this.GetHitpoints()? Only adds performance cost without adding benefits, doesn't it?

Thu, Apr 25, 1:32 PM
wraitii added a comment to D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).

Will update with your comments, except for the "runspeedmultiplier" thingy.

Thu, Apr 25, 1:29 PM
wraitii added a comment to D1847: Optimise out of frustum rendering of textures overlays.

I think perf impact is negligible (it's definitely better, but probably not by much, and the main impact is likely a lower number of draw calls).
Probably a good idea to do it anyways though.

Thu, Apr 25, 12:29 PM
wraitii added inline comments to D1754: Pass an argument to GUI events / SendEventToAll, remove loading screen progess workaround.
Thu, Apr 25, 11:58 AM
wraitii added a comment to rP22225: Fix D1491 which introduced an ENSURE that should not have been there..

It is indeed... We don't use the jpc at the moment so this doesn't crash, but it should be fixed.

Thu, Apr 25, 10:15 AM
wraitii added a comment to D1491: Const-Correct the long range pathfinder.

Yup, that ENSURE was broken... Mind you my initial code was _also_ broken since at() would have failed here too. Apparently I didn't playtest that revision and we have no tests calling ComputeJPSPath, which we really ought to (might add some this WE).

Thu, Apr 25, 9:09 AM
wraitii committed rP22225: Fix D1491 which introduced an ENSURE that should not have been there..
Fix D1491 which introduced an ENSURE that should not have been there.
Thu, Apr 25, 9:08 AM
wraitii added a comment to D1491: Const-Correct the long range pathfinder.
Thu, Apr 25, 9:00 AM

Wed, Apr 24

wraitii added inline comments to D1584: Seed random sounds.
Wed, Apr 24, 9:53 PM
wraitii committed rP22219: Const-Correct the long range pathfinder.
Const-Correct the long range pathfinder
Wed, Apr 24, 9:07 PM
wraitii closed D1491: Const-Correct the long range pathfinder.
Wed, Apr 24, 9:07 PM
wraitii committed rP22218: Hierarchical pathfinder: fix an issue with regions and some touch-ups.
Hierarchical pathfinder: fix an issue with regions and some touch-ups
Wed, Apr 24, 9:02 PM
wraitii closed D1832: Hierarchical pathfinder: fix an issue with regions and some touch-ups (D53 outtake).
Wed, Apr 24, 9:02 PM
wraitii added a comment to D1804: Fixes black water glitches for certain wind angles.

Foam really isn't that useful anymore either, so I guess commit anyways.

Wed, Apr 24, 8:13 PM
wraitii added a comment to D1804: Fixes black water glitches for certain wind angles.

I think I'll check out how that changes because that might have been WAD by me tbh. Up to personal preference I guess.

Wed, Apr 24, 3:05 PM
wraitii added a comment to rP22207: Actually add the test file for rP22205..

Thanks @Itms , @Stan and @vladislavbelov

Wed, Apr 24, 8:38 AM
wraitii accepted D1804: Fixes black water glitches for certain wind angles.

Mind you, foaminterp.x == foaminterp.z. So it equals to:

foam1.x = foaminterp.x * (WindCosSin.x - WindCosSin.y);
Wed, Apr 24, 8:37 AM

Tue, Apr 23

wraitii added a comment to D1482: Target build version explicitly for Xcode.

I think I also changed build/workspaces/update-workspaces.sh

Tue, Apr 23, 7:41 PM
wraitii added a comment to D1772: Fix TLS Segfault on various mac versions.

So you acknowledge that the commit may actually be wrong and for instance obfuscate instead of resolve the issue, or fix the issue in one case but still come with a configuration that we don't want, or possibly even breaking under some circumstances, but you are just not concerned about that?

Tue, Apr 23, 2:36 PM
wraitii added a comment to D1772: Fix TLS Segfault on various mac versions.
In D1772#75868, @Itms wrote:

The issue might be in our code (and it might not) but I don't have time to debug it properly and/or won't do it anyways as I already have plenty of things I care to do on this project.

I agree with Itms, if you don't have the time or will to do something properly, then don't do it.

Yeah I'm not sure why you went ahead. I'm happy it's in, but you could have left it aside if you wanted, and it would have been thoroughly reviewed in the same way as the other macOS TLS patches from the re-release (probably at a moment where it would become high priority, like the A24 release). Did you feel pressured to review it somehow (this is what your message looks like)? Or did it look like a low hanging fruit? I'm interested in knowing that in order to make sure the priority order of reviews in done in a sensible way.

Tue, Apr 23, 2:30 PM
wraitii added a comment to rP22207: Actually add the test file for rP22205..

Well whoever gets to test it first commits the fix I'd say 👍

Tue, Apr 23, 11:04 AM
wraitii added a comment to rP22207: Actually add the test file for rP22205..

Any chance you can test that?

Tue, Apr 23, 10:19 AM
wraitii added a comment to rP22207: Actually add the test file for rP22205..

Annoying... Can anyone raise a concern? I'm not sure I'll have time to fix it today/tonight - also don't have VS13. I'll have time tomorrow if needed.

Tue, Apr 23, 8:47 AM

Mon, Apr 22

wraitii added inline comments to D1844: Improvements to simulation hotloading before the SM upgrade.
Mon, Apr 22, 11:46 PM
wraitii added inline comments to rP22205: Add tests for the hierarchical pathfinder..
Mon, Apr 22, 11:44 PM
wraitii added inline comments to D1491: Const-Correct the long range pathfinder.
Mon, Apr 22, 11:12 PM
wraitii added inline comments to D1844: Improvements to simulation hotloading before the SM upgrade.
Mon, Apr 22, 11:10 PM
wraitii added a comment to D1844: Improvements to simulation hotloading before the SM upgrade.

I'm OK with the reasoning. Some comments.

Mon, Apr 22, 11:10 PM
wraitii requested changes to D1333: Precompiled update.

Rm, on second thought, I think we need to keep the original headers in most files. For one thing this would reduce the number of changes (which helps with rebasing old patches and is generally nicer) and for another thing I need to look at what you're actually putting in the precompiled headers. Some of them might increase build times from scratch but cause a bit of pain when modifying these files...

Mon, Apr 22, 7:13 PM
wraitii updated the diff for D1491: Const-Correct the long range pathfinder.

Should run on top of rP22210.

Mon, Apr 22, 6:17 PM
wraitii added inline comments to D1832: Hierarchical pathfinder: fix an issue with regions and some touch-ups (D53 outtake).
Mon, Apr 22, 6:13 PM
wraitii updated the diff for D1832: Hierarchical pathfinder: fix an issue with regions and some touch-ups (D53 outtake).

Reupload after dependent commits have been merged.

Mon, Apr 22, 6:13 PM
wraitii committed rP22210: Const-correct the hierarchical pathfinder..
Const-correct the hierarchical pathfinder.
Mon, Apr 22, 6:07 PM
wraitii closed D1830: Const-correct the hierarchical pathfinder (D53 outtake).
Mon, Apr 22, 6:07 PM
wraitii committed rP22209: Fix portrait icons of mechanical units broken by rP22204. Reported by Polakrity..
Fix portrait icons of mechanical units broken by rP22204. Reported by Polakrity.
Mon, Apr 22, 5:53 PM
wraitii added a comment to rP22204: Rename generic ballista template to boltshooter and onager to stonethrower to….

Mh, didn't think of running that script indeed. I'll fix it, thanks.

Mon, Apr 22, 5:39 PM
wraitii added a comment to rP22208: Accidentally committed an upstream version of the hierarchical pathfinder tests….

...Because of course I did...

Mon, Apr 22, 4:34 PM
wraitii committed rP22208: Accidentally committed an upstream version of the hierarchical pathfinder tests….
Accidentally committed an upstream version of the hierarchical pathfinder tests…
Mon, Apr 22, 4:31 PM
wraitii added inline comments to D1836: Expose Hierarchical Pathfinder's reachability functions (D53 outtake, D13 prerequisite).
Mon, Apr 22, 4:02 PM
wraitii added inline comments to D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).
Mon, Apr 22, 3:22 PM
wraitii added a comment to D1830: Const-correct the hierarchical pathfinder (D53 outtake).

This is next on my commit list (once builds have passed for the rest) as it is needed for D1832 to cleanly apply, and I'll push new tests proving D1832 then, then commit that, which enables running D1834 on top of all these tests, which I then feel reasonably confident to commit soon (I'd like to run an MP replay with it to ensure hashes are consistent though).

Mon, Apr 22, 2:47 PM
wraitii committed rP22207: Actually add the test file for rP22205..
Actually add the test file for rP22205.
Mon, Apr 22, 2:44 PM
wraitii added a comment to rP22205: Add tests for the hierarchical pathfinder..

There was... Thanks for noticing.

Mon, Apr 22, 2:43 PM
wraitii added a comment to rP22206: Add a few assertions to pathfinder tests, fix the disabled tests..
In rP22206#32931, @Stan wrote:

You fixed disabled tests but did not enable them ? :)

Mon, Apr 22, 2:42 PM
wraitii committed rP22206: Add a few assertions to pathfinder tests, fix the disabled tests..
Add a few assertions to pathfinder tests, fix the disabled tests.
Mon, Apr 22, 2:14 PM
wraitii closed D1831: Some more pathfinder tests (D53 outtake).
Mon, Apr 22, 2:14 PM
wraitii committed rP22205: Add tests for the hierarchical pathfinder..
Add tests for the hierarchical pathfinder.
Mon, Apr 22, 2:09 PM
wraitii closed D1833: Hierarchical Pathfinder tests (D53 outtake).
Mon, Apr 22, 2:08 PM
wraitii requested changes to D1346: List multiselection.

I would suggest that C++ GUI lists are always "multi select", but we can enable/disable the actual multi-selection part. So change 'selected' to be a CIntList, which would return a one-sized array. Then JS code wouldn't have to awkwardly handle two cases.
That would imply modifying a lot of existing JS GUI code though. But it sounds greppable. So I'd say give it a shot, if it's too difficult I'll change my mind.

Mon, Apr 22, 1:32 PM
wraitii requested changes to D1702: GUI addon to animate objects proprieties.

TBH this is too complex and too complicated for my tastes as it's written. I say 'remove stuff'.

Mon, Apr 22, 1:20 PM
wraitii updated the diff for D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).

Following elexis' comments (I'll link this one but see the whole thread), adjust variable names for clarity:

Mon, Apr 22, 12:36 PM
wraitii added a comment to D1510: Update to Spidermonkey 45.0.2.

@Itms Hm, so what's the status on this?

Mon, Apr 22, 9:40 AM
wraitii added reviewers for D1752: Add GUI events for middle mouse click: Restricted Owners Package, Restricted Owners Package.
In D1752#70523, @elexis wrote:

There will certainly be people with only 2 buttons (Wasn't that about every mac user?)

Nowadays mac users have no buttons at all and touch input, in fact ;)

Mon, Apr 22, 9:38 AM
wraitii added reviewers for D1346: List multiselection: Restricted Owners Package, wraitii.

Might also take a look at this.

Mon, Apr 22, 9:37 AM
wraitii added a reviewer for D1631: TileClass optimization.: Restricted Owners Package.
Mon, Apr 22, 9:36 AM
wraitii added reviewers for D1817: Remove unused variable and conversion in the ComponentManager: wraitii, Restricted Owners Package.
Mon, Apr 22, 9:35 AM
wraitii added a reviewer for D1703: Map browser for gamesetup: Restricted Owners Package.
Mon, Apr 22, 9:35 AM
wraitii added reviewers for D1777: Move map data to settings.js: Restricted Owners Package, wraitii.
Mon, Apr 22, 9:34 AM
wraitii added a reviewer for D1650: GridBrowser GUI addon: Restricted Owners Package.
Mon, Apr 22, 9:33 AM
wraitii added a reviewer for D1702: GUI addon to animate objects proprieties: wraitii.
Mon, Apr 22, 9:33 AM
wraitii added a reviewer for D1825: Resize (XML object) bar JS GUI addon: Restricted Owners Package.

This does sound like a 'core' c++ feature to me, but having a JS implementation first and then possibly porting it to C++ sounds like an interesting approach to me.

Mon, Apr 22, 9:30 AM
wraitii requested changes to D1815: match unit classes with templates.

Care to do the siege units as well?

Mon, Apr 22, 9:24 AM
wraitii added a reviewer for D1775: Delete unused technologies: Restricted Owners Package.
Mon, Apr 22, 9:23 AM
wraitii added a reviewer for D1838: Add proximity attack component.: Restricted Owners Package.

Open question for other devs: don't we already have a patch for trample damage somewhere?

Mon, Apr 22, 9:23 AM

Sun, Apr 21

wraitii added inline comments to rP22203: Communicate field diminishing returns to the player in the fields tooltip.
Sun, Apr 21, 8:46 PM
wraitii added a comment to rP22204: Rename generic ballista template to boltshooter and onager to stonethrower to….

I'm wondering the real usefulness of the bolt shooter vs stonethrower templates also. It seems to me basically all "shooter" siege engines of the time were torsion engines, which could fire smaller or bigger projectiles and arrows or stone sort of indifferently.
The main difference was mostly whether they could shoot precisely (aka be used as a marksman's tool) and mostly horizontally to target enemy units, which was in general a factor of size it seems.
Might be more interesting to have a "light" and "heavy" variant or something. Or just merge them and specialize derived templates since siege engines were kind of civ-specific beyond rams.

Sun, Apr 21, 7:18 PM
wraitii added inline comments to D438: Change Run Speed into a Run multiplier.
Sun, Apr 21, 7:08 PM
wraitii committed rP22204: Rename generic ballista template to boltshooter and onager to stonethrower to….
Rename generic ballista template to boltshooter and onager to stonethrower to…
Sun, Apr 21, 7:04 PM
wraitii closed D1760: Remove template_unit_mechanical.xml.
Sun, Apr 21, 7:04 PM
wraitii added a comment to D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).
In D1840#75562, @elexis wrote:

If the value that SetSpeedRatio computes differs from the serialized value, by definition it's OOS?

Sun, Apr 21, 6:42 PM
wraitii added inline comments to D438: Change Run Speed into a Run multiplier.
Sun, Apr 21, 6:41 PM
wraitii added a comment to D438: Change Run Speed into a Run multiplier.
In D438#75552, @elexis wrote:

(See also comments in rP22197.)

But m_SpeedRatio is serialized and then replaced by m_SpeedRatio = std::min(m_SpeedRatio, m_RunSpeedMultiplier);.
This sounds like it would go OOS if that line changes the value,
or if the line never changes the value, is a useless line (misleading the reader to believe that it has use)?

See D1840, but this is only to check if the "new" m_RunSpeedMultiplier isn't actually lower than m_SpeedRatio, which can happen after an aura/tech/ownership change.
Deserialization actually uses this call solely to recompute m_Speed.

Sun, Apr 21, 6:06 PM
wraitii added inline comments to D1828: Health.js cleanup: add tests, add an "IsInjured" function, use GetHitpoints everywhere.
Sun, Apr 21, 6:00 PM
wraitii updated the diff for D1829: Make Pathgoal::NavcellContainsSquare use only the top/left edge of the square (D53 outtake).

Mhpf, you're right. Fix other whitespace in this file while I'm at it.

Sun, Apr 21, 5:59 PM
wraitii added a comment to D438: Change Run Speed into a Run multiplier.

See D1840.

Sun, Apr 21, 5:54 PM
wraitii created D1840: RunSpeedMultiplier cleanup: improve variable names and reuse a duplicated function (rP22197).
Sun, Apr 21, 5:53 PM
wraitii added a comment to D438: Change Run Speed into a Run multiplier.

This code is safe.

Sun, Apr 21, 5:46 PM
wraitii updated the diff for D1828: Health.js cleanup: add tests, add an "IsInjured" function, use GetHitpoints everywhere.

Remove the helpers mock for now.

Sun, Apr 21, 5:33 PM
wraitii updated the diff for D1829: Make Pathgoal::NavcellContainsSquare use only the top/left edge of the square (D53 outtake).

Hopefully clarify comments.

Sun, Apr 21, 5:11 PM
wraitii added a comment to D1837: Input.js refactoring - separate diplomatic tribute in their own file..

About creating new files, Im skeptical if they are 120 lines short, since the other files are couple of thousand lines long

Sun, Apr 21, 5:06 PM
wraitii added inline comments to D1839: Fix hotkey event sync with hotkey state..
Sun, Apr 21, 4:23 PM
wraitii created D1839: Fix hotkey event sync with hotkey state..
Sun, Apr 21, 4:19 PM
wraitii added a comment to D1837: Input.js refactoring - separate diplomatic tribute in their own file..

@elexis I'm inviting you to look at this, from playing around with it it feels much nicer to me.

Sun, Apr 21, 11:11 AM
wraitii added inline comments to D1837: Input.js refactoring - separate diplomatic tribute in their own file..
Sun, Apr 21, 11:08 AM
wraitii updated the diff for D1837: Input.js refactoring - separate diplomatic tribute in their own file..

More complete example of what I'm envisioning. We'd have possibly multiple "Input State" components that listen to events and possibly 'kill' themselves - so no transition from an input state to another input state except "no input".

Sun, Apr 21, 11:05 AM

Sat, Apr 20

wraitii added inline comments to D1829: Make Pathgoal::NavcellContainsSquare use only the top/left edge of the square (D53 outtake).
Sat, Apr 20, 7:35 PM
wraitii added inline comments to D1828: Health.js cleanup: add tests, add an "IsInjured" function, use GetHitpoints everywhere.
Sat, Apr 20, 7:35 PM
wraitii added a comment to D1691: Remove boost "system" from Mac OS build system..

I feel extremely "meh" about this :p
Commit it if you want, don't if you don't. It's unlikely I'll do it myself is my point.

Sat, Apr 20, 5:51 PM
wraitii committed rP22200: Remove workaround in GetGUIObjectByName.
Remove workaround in GetGUIObjectByName
Sat, Apr 20, 5:49 PM
wraitii closed D1701: GUIManager m_CurrentPage workaround removal.
Sat, Apr 20, 5:49 PM
wraitii committed rP22199: Cleanup deprecated SM-specific syntax in ExtractFormations in Commands.js.
Cleanup deprecated SM-specific syntax in ExtractFormations in Commands.js
Sat, Apr 20, 5:35 PM
wraitii closed D1724: Cleanup ExtractFormations in Commands.js.
Sat, Apr 20, 5:35 PM
wraitii added a comment to D1804: Fixes black water glitches for certain wind angles.

Wouldn't it be better to max(0, ...) the whole calculation? This is magic-graphical-code but you're changing behaviour a lot here.

Sat, Apr 20, 5:30 PM
wraitii updated the diff for D1074: Walk to target rather than walk to point.

Remove the "only if blocking pathfinding" because I don't see the point tbh.

Sat, Apr 20, 5:26 PM
wraitii commandeered D1074: Walk to target rather than walk to point.
Sat, Apr 20, 5:25 PM
wraitii added a comment to D665: Make ship pickup nicer.

At this point it'd probably be usefully rebased on top of D1834... I'll keep this in my review queue as I might commandeer at some point in the future.

Sat, Apr 20, 5:16 PM
wraitii added a comment to D1691: Remove boost "system" from Mac OS build system..

I'm not sure how useful this is. We're linking statically anyways and this is OSX only so...

Sat, Apr 20, 5:14 PM