- User Since
- Dec 21 2016, 1:38 PM (258 w, 2 d)
Sep 29 2021
Experimentally, unfortunately :P
The number only really work for standard sized circle maps I think.
Sep 28 2021
Er, we technically depend on a few persons now. If the servers were to crash irrecoverable and backups didn't work, we'd lose everything as well. Point is, I don't think this is much of an argument either way, though there are pros & cons for both.
Phab will keep running as long as we keep it running on our servers. Also I saw there were efforts to fork it.
AFAIK none support SVN, unfortunately. And well, it's true, but it'll also keep all its security flaws (if any) & its general slowness & never get any new feature.
I think this is absurdly unlikely at this point, but even if GitHub deletes the repose unilaterally, git makes it so that we'd very likely have recent-is mirrors on several dev computers & on the lobby server itself. I don't think this should be a concern.
Why give away the sovereignty we have?
'Cause Phabricator is dying, see https://wildfiregames.com/forum/topic/42181-phabricator-is-no-longer-being-maintained-upstream/
Also sovereignty is essentially kept per the first point.
Why let the codebase fall apart, literally?
- It's a good way to gain knowledge about GitHub usage, which will help should we decide someday to migrate somewhere else.
- the person who wants to work on this code wants to work on GitHub because it's easier for them
Say if I were to implement a new system of ratings, I did need to change the bots (as I need to compute the ratings) as well as some gui (i.e. I need to show the new rating). Having several repo's makes developing such a feature much harder, so is not wanted.
It's a quite realistic prospect to change the rating calculation without changing the rating display, and vice-versa.
The actual hard problem is deployment, and since we don't actually deploy the bots synchronously with SVN, we'd already have a lot of trouble if you wanted to implement a new system of rating.
Sep 23 2021
This is getting off-topic, but maybe the right thing to do is make double-clicking select the control group instead of all similar units, if the entity is in a control group?
I think you should merge this, it's a nice improvement & I'm sufficiently available to fix the CI if need be.
@Langbart: Check that it works with non Kushite units but if so yes.
I believe Vlad's point is that you could actually just query 4 height map points and draw a more accurate polygon. Though by itself the polygon would still be somewhat inaccurate because the field of view is not a perfect rectangle on hilly maps. The cost of doing that is likely not much worse than doing one query.
Yeah using formations for anything but walking / patrolling is various kinds of broken, which is why the default setting is not 'no override'.
Sep 20 2021
Well, a new user wouldn't have many mods, but point taken.
There is also a filter valid mods checkbox in the download screen. If anything it should be consistent.
I really only see one usecase of the other default: if there are no compatible mods available. However in that case it would be much better to write that instead of the list of available mods.
How about I just adding "there are hidden, incompatible mods" as the last line in all cases where there are some
[disclaimer -> not tested]
I think TaskedResourceType might be a better fit than 'current'
Sep 17 2021
You do have to compile again, after you applied the patch. Then it should work.
mh, this diff doesn't change UnitAI, so I'm not entirely sure why you had a problem in UnitAI?
Sep 16 2021
This seems like a good idea 👍
I think you're doing redundant work on INVALID->Player when then entity spawns, which is bad as that's quite slow already.
Sep 9 2021
For what it's worth, this was intended to make upgrade less good, as it's currently very efficient since it takes no builder (something that e.g. Nescio didn't like in the Tower case IIRC). Giving a temporary debuf allows balancing the cost in resources & build time vs the ultimate bonus. Whether the current balance is good is quite debatable.
I kinda like the approach of a 3rd-party positioning class. I had some reserves about overcomplicating layout logic, but it doesn't seem bad here.
Sep 8 2021
Looks to me rather unlikely that'd we'd get them to move quickly on this? Maybe they have an issue tracker where you can report that.
You can call super with the argument to the GUI object though? Maybe I'm missing something in your comment?
You're duplicating the code to fetch the caption size & align three time, which suggests that perhaps those 3 components should inherit from a common class (Chat Text Input?). GetTextWidth is definitely a better approach however :)
Sep 7 2021
If you've failed to find a system-wide setting, I think we just need to strip the email manually. To be honest, I'm wondering how relevant it is to bundle the translator names at all in the .po files, but the email is definitely a larger issue.
Sep 6 2021
This is definitely a false positive, see https://code.wildfiregames.com/source/0ad/browse/ps/trunk/source/network/StunClient.cpp$337
Sep 4 2021
Debatable. One could equally expect the setting to be purely graphical (and I would argue one is more likely to do that based on "RTS history", but that's also debatable).
Sep 3 2021
Aug 30 2021
It seems like Stan is only moving the demo maps. Which isn't super useful IMO, since those are already hidden by the 'demo' tag. Moving lower-quality maps is a different topic, which IIRC was discussed as 'community maps' but the naming really isn't that good.
Aug 29 2021
Your test plan is missing the MP case and I think this is where things are liable to get broken, and so I think a few changes would improve this diff.
Aug 27 2021
My main question is 'how necessary is this' versus regular placers with high enough repetitions?
My understanding is that the most relevant piece is the code that picks the area surrounding a player base, but that's just a "neighborconstraint" (can't remember the proper name, think it's not that) large enough, no?
IMO, would be better on the bottom right side, overlapping a bit with the icon. I dislike the pure red, and would rather just use white.
Aug 26 2021
I believe we don't have a notion of Idle for buildings, since they don't use unitAI, hint hint @Freagarach ;).
This this is a good idea 👍
Aug 25 2021
err, had misunderstood the problem, indeed code bug. Weirdly, did it correctly for seeds. Meh.
I also don't return there so I guess fine.
The lobby bot behaviour seems bit odd to me, but I don't see why this wouldn't work.
I believe I generally return true in pickRandomItems (or false, don't remember which means "I'm done").
I might be completely off-base here but I believe there _was_ a reason for not having MP messages in the lobby. That being said, I don't remember which exactly.
Be careful that this doesn't break behaviour, IIRC I had some issues with formations having low turn rate on A25.
Thanks for the patch :)
Quick glance looks good, I guess I'll formally review & merge this when I get back to some more activity. No ETA unfortunately.
Aug 23 2021
Aug 12 2021
Aug 9 2021
Think I failed to notice that because that's always been broken on mac OS :/
Aug 8 2021
Aug 7 2021
I've given some more thoughts to this and I'm not sure it's a good idea _by default_, at least for larger changes. It can break some maps, and it makes the maps less unique if the biomes determine more stuff.
Aug 3 2021
Looks alright - haven't tested.
I think the imperative form looks fine. I'd like to see a comment added that FindWalkAndFightTargets must be called for all members or something, since that's not actually so trivial by reading I think (hence my original mistake).
Freagarach means using token modifiers to change the templates, instead of adding new templates. You can look at the tests I think for some examples.
Aug 2 2021
Jul 31 2021
Fixed in rP25843
Fix mistake noted by Vlad
And here it is in A23, showing that the fixed version is indeed correct wrt to the original, working implementation:
Oh and BTW here's the same spot in A24, confirming that the bug was indeed present there already, just hidden on Ngoro for whatever reason:
Note also that this effectively applied a MAX 1D convolution on the X-axis, so the heightmaps generated now are slightly finer/more accurate. Here's the change on Red Sea (Giant):
(NB: screenshots not taken from a replay so the rest of the map differs slightly)
(Note that on smaller map sizes, because of scaling, the difference is less noticeable).
That's normal, because it reloads the whole actor, and so since you removed a variant possibly the random number generator is doing things differently, thus different sword.
Adressing a few things:
[18:55:02] <elexis1> transforming to TEX_BGR resulting not in BGR is not a bug?
The 'spec', if there is any, is that grayscale doesn't convert to BGR/BGRA. Not a bug per se, just possibly surprising behaviour.
[18:56:24] <elexis> the texture library changed?
[18:56:37] <elexis> because we never had that bug in a23
It must have happened, but was either impossible to detect / not harmful as it only really shows up if something tries to read the height values 'off' the map (which the smoothing on ngorogoro does now).
[18:57:01] <elexis> so I assume it wasnt in a24 either
[18:57:14] <Freagarach> Correct.
Again, I don't think we've actually confirmed that - but if it's the tile class change, that's just a bug hiding another bug as usual.
[18:58:06] <elexis> and if converting to BGR doesnt result in converted to BGR then that might trigger bugs elsewhere too it sounds like
[19:09:30] <elexis> if that is a bug, then the rmgen code is correct and the diff would hide it
The rmgen code is incorrect by the sheer fact that it has a variable to handle different bytes-per-pixel, but doesn't actually do that. Either hardcode 4 and actually ensure it's BGRA, or fix the rmgen code. The latter is vastly more efficient for grayscale images, which are most height maps, so was my solution here.
[20:11:45] <elexis> the CODEC_CANNOT_HANDLE should be caught then and prevented
I don't necessarily disagree with that statement, but the texture loading code is dangerous stuff to touch.
Jul 30 2021
Removing the 'debug' L90 breaks the code for grayscale images (which are all our height maps at the moment). The reason is subtle and annoying: the code attempts to convert to uncompressed BGRA, so presumably a bytesPP of 4. However, for grayscale images, this conversion fails silently with a CODEC_CANNOT_HANDLE when it tries to add the alpha channel (it would fail loudly when converting to BGR). Thus the image retains a bytesPP of 1. Thus the code is invalid: mapdata[offset] is OK, but the other two are reading other pixels, and in the case of the bottom-right pixels, they're actually reading random garbage.