- User Since
- Mar 7 2017, 11:03 AM (215 w, 16 h)
May 30 2020
The changes you wanted are wrong and your attitude is wrong. Planning for a hypothetical future "roc" unit? Just let the code do what it was written and tested to do, don't delay it even more because of imaginary features that don't exist yet and may never, and probably shouldn't share the same code even if they did. And you may well cause a performance issue by taking out the timer. Just commit the fucking patch. It works. It has always worked through 3 years of delays and irrelevant changes.
I have had enough. Years of inaction on this obviously necessary bug fix to an extremely annoying part of the game which was created on Jun 20 2017, this now being May 29 2020. Finally a few comments. So I have hope, I spend my time making some requested changes. Then, nothing again. This community is broken. If anyone wants D665, and wants any more changes made, commandeer the patch yourself. But honestly none of these recent changes were functionally necessary anyway, which is another broken part of the community. Even the first revision of this patch, 3 years ago, without the long pathfinder stuff, was fully working and better than the status quo which is still broken. Just commit the shit and make the game work. Your functionally useless whims and preferences should not become time demands on the patch creator.
May 22 2020
I renamed to CheckPickupAccessible because it's actually used for siege pickups as well as ship pickups.
rename CheckNearShore to CheckPickupAccessible, and also test the square directly under the ship.
May 18 2020
May 17 2020
You're right. I have changed the patch to do that. When I first wrote the patch, GetGlobalRegion didn't exist.
Dec 4 2019
Dec 3 2019
Fix LeaveFoundation unpacking behavior. This required some refactoring to avoid code duplication. As a result, all of LeaveFoundation behavior (siege unpacking, ordinary units, units in formation, animals) needs testing now.
split precondition test
Dec 2 2019
Dec 1 2019
Remove the extra bugfix that was split into D2174.
Update to trunk. Slightly refactor (not a functional difference from the patch before).
That's what the patch already does.
Updated to trunk. Took out the std::unordered_set change, because other code in HierarchicalPathfinder.cpp now depends on the set being ordered.
Jan 17 2019
Jun 2 2018
Yeah, it's not that it wasn't tested, it's that the test coverage wasn't good. My method of testing disconnects was to kill -9 the client process, which for some reason results in a longer time before disconnect if applied during game loading with the earlier patch version (~40s) than the method of setting artificial lag with tc. Also tested rejoins, where it was working fine. And we didn't ever manage a test with a real dropping player.
Jun 1 2018
Tested, works. The message is still being sent while the client is on the loading screen, but it's hidden there and causes no problems.
May 30 2018
add config option
increase timeout to 80s
In practice, this patch allows about 60 seconds of timeout. I'm not sure on exactly why but that's what local testing shows.
Increase loading timeout from 40s to 80s
May 28 2018
May 27 2018
Added code to cancel packing/unpacking the unit as necessary when it is issued an attack order, depending on the range to the target. This code is not pretty due to having to work around the way UnitAI handles packing.
May 26 2018
May 25 2018
From what I know about pushing in SC2, stationary units on a task like attacking (or gathering, although that case does not arise in SC2 because gatherers ignore collisions) do not get pushed at all. There are rules about which units can push which other units.
May 24 2018
May 23 2018
add periods to comments
May 22 2018
Whitespace fix. Also, I'm now confident through local testing that the patch works.
May 21 2018
elexis comments and don't change timeout on a local client
Apr 28 2018
remove the mace/ptol template fix from the patch
Apr 27 2018
I think that grouping packed and unpacked catapults together is better than the current behavior. The player can triple click if they want the old behavior. I've noticed in many of my games that I frequently want to retreat *all* catapults because some anti-catapult thing is approaching, and it's a minor annoyance that double clicking doesn't select them all. I don't think wanting to select all packed or all unpacked is as common a use case. If I only wanted to move certain catapults I would drag to select them.
Added a SelectionGroupName for siege towers as well
The other catapult and bolt shooter templates don't have this problem.
Also added a SelectionGroupName to catapults and bolt shooters
Feb 6 2018
Dec 23 2017
Dec 19 2017
They're not related in the code, but the word "overlay" does describe them. They are things that are painted on top of (overlay) the game simulation.
Dec 18 2017
You requested a comment on IRC: I would prefer the "Overlays" category because it's more clear what goes into it. A "Gameplay" category could include more options than those you listed: windowed mode, detailed tooltips, all sound and graphics settings, and even chat notification settings influence gameplay. It's overbroad.
Dec 13 2017
Looks good. Although I don't think it's necessary to have a separate buffer range. If the alert range (140m) doesn't include all the desired garrison buildings, you could just increase the alert range a bit. I think 140m should be enough for all alerted units and buildings.
Dec 12 2017
But you could also manually ungarrison siege into shallow water in that situation, right? Also, it's questionable how many people at the time could actually swim. Particularly, a hoplite in armor would have probably drowned even if he knew how to swim. The more important point is that no unspoiled player is going to expect this behavior or have any idea what happened when some of his units disappeared. If there's a ship with a mix of infantry and cavalry, and he's used to infantry automatically ungarrisoning when ships sink, he's more likely to assume there wasn't enough space to ungarrison or the ship wasn't close enough to shore, than to assume there is some special rule that cavalry can't swim.
Horses can swim. Anyway it only ejects the units if the ship is already at the shore, i.e. when any unit including siege could have just ungarrisoned normally, so it's hard to make a case that they would have to swim.
IMO ships should eject everything on destroy. To only eject some unit types is a hidden mechanic that's not intuitive or easy for the player to figure out, which has little effect on gameplay except when the player expected things to be ejected but they weren't.
I get a conflict applying the patch, can you update to svn?
rejected hunk @@ -1,162 +1,135 @@
Looks good except for one thing: if you raise an alert and a unit tries to garrison beyond the alert range, and then you cancel the alert before the unit finishes garrisoning but after the unit has walked beyond the alert range, it will continue to garrison. My suggestion would be to filter the list of buildings a unit may garrison in, to exclude buildings located beyond the alert range from the alert raiser. With that change, when you end the alert, you only need to check buildings out to the alert range instead of alert range + search range.
Dec 8 2017
Aug 19 2017
It's not fixed in SDL 2.0.5, we'd need to submit a patch to a future SDL version. Ubuntu 17.04 does come with SDL 2.0.5.
It's hardcoded by SDL in SDL_mouse.c: SDL_double_click_radius = 1. https://fossies.org/linux/SDL2/src/events/SDL_mouse.c There was some discussion on changing the value here: https://forums.libsdl.org/viewtopic.php?p=51852. Perhaps someone could submit a patch to SDL to increase the default radius and add a setter.
Jul 6 2017
On second thought it seems you can't select a mix of own and ally buildings. There is also a justification for keeping unload-all-by-owner and unload-all as separate commands because petra uses unload-all-by-owner.
Jul 1 2017
Account for a couple things:
Jun 26 2017
If a building is captured or destroyed, ejected units will try to find a new building to garrison inside.
That may not be a good thing. I often will set a rally point for a building before it's destroyed, so that the units inside will run away to the desired location when the building is destroyed.
replaced set with unordered_set (this required it to also be changed in many places in HierarchicalPathfinder)
Fix an additional (pre-existing) bug discovered in UnitAI where if you give the order to garrison the same unit in two ships in a row, the second ship will not pickup.
Jun 24 2017
"The bug" in this context is where the ship moves away from the shore when the user wants to put something in it. This specific gameplay issue doesn't need the solution you're describing. Finding a pickup location is separate from shoreline detection, and a complete pickup solution will need both (see below), so this patch is a step forward in any case. I also want to note that mimo only asked for connectivity based shoreline detection, which we now have, so let's move forward on that.