- User Since
- Mar 7 2017, 11:03 AM (139 w, 6 d)
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.
I have updated the patch so that it uses the hierarchical pathfinder to determine whether the ship is on a shore that the unit to be picked up can reach.
Jun 22 2017
Are you going to do the fix yourself? It has been broken for 3 years and no one has fixed it.
Just disable pickup.
Jun 21 2017
IMO it's straightforward, not a hack - the previous code had a test intended to check whether the ship was on the shore, which was incorrect and even had a commented TODO to fix it, and now it has been fixed. The code correctly detects when the ship is at the shore, does it not? A cliff is still "shore."
Here's a thought: if you want to make sentry towers effective against cavalry raids in general (not just camels) then you could decrease their cost. If they cost 50 wood and had 20 build time (-50% cost and build time) then they would be cost effective to build in response to a cavalry raid. Even though they wouldn't be enough to stop the raid by themselves they would be worth building. There's little risk of imbalance in this case, because if they had 0 cost and you just started the game with your territory full of sentry towers, enemies could still raid. Compare that to Iberian walls - a territory full of sentry towers is much weaker defensively than the Iberian starting walls.
Looks fine. I don't think that sentry towers are any use against cavalry raids in a normal non-pizza game though. (Maybe they could help against camels.) You need to make your own cavalry, and spearmen.
fix GarrisonHolder unit type lists
Jun 20 2017
Jun 16 2017
remove details from comment
If you want me to review this, I need to test them... so either remove me as a reviewer or add a test plan that at least covers each line of code touched. There are some things here I'm not 100% sure about. I'm not entirely certain how scoping works here with respect to g_LastTickTime, which you define in two files. Does that mean one overwrites the other? Or are they in different contexts? Other than that I have looked over the code and it looks OK to me, so if that's all you were looking for you can commit, but I'm not going to put a green checkmark on this without testing.
added a comment
Fix it in a different way that also fixes the bug with INPUT_PRESELECTEDACTION and is more resilient to future changes.
elexis agreed it would be best for me to commandeer this
Jun 15 2017
Not sure how to attach another patch to the revision. I would suggest this change instead of the current one:
Index: binaries/data/mods/public/gui/session/input.js =================================================================== --- binaries/data/mods/public/gui/session/input.js (revision 19728) +++ binaries/data/mods/public/gui/session/input.js (working copy) @@ -831,6 +831,9 @@ updateAdditionalHighlight(); }
This doesn't cover all cases. If you click on the unit and give it a preselected action (repair, patrol, guard) and double click as you perform the preselected action, you will see the same bug. The core problem is that clickedEntity is associated with a click event and any double or triple clicks that follow it, but is being retained for the next click event. It's supposed to get reset but that only happens if the next single click event's inputState is INPUT_NORMAL
Jun 2 2017
Well, in that case it would be "the Ford Mustang." "I read Gulliver's Travels" or "I read the Gulliver's Travels"? (the first one). "Alice lives in Paris" or "Alice lives in the Paris"? English isn't super consistent. Sometimes you would use "the" before a proper noun, and sometimes you would not. You might find https://english.stackexchange.com/questions/59271/why-there-is-the-before-some-names-but-not-others interesting/horrifying.
Jun 1 2017
May 23 2017
I give up. Just a last note: check that Alexander actually does affect the cc with his short 20m range. The center of the CC might be out of his range.
used a constant for 100ms