Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Place default rally point of WEAP, BARR and TENT closer to exit #16701

Closed
wants to merge 1 commit into from

Conversation

netnazgul
Copy link
Contributor

@netnazgul netnazgul commented Jun 16, 2019

There is a long-standing issue with rally points in that if they are placed into an impassable cell, units do not go to the location and instead just exit the production facility and stop there. Particularly annoying consequence of that is that if you place War Factory close to a building, a cliff or a tree, thus overlapping the default rally point into an impassable cell, Ore Trucks exiting that War Factory will drop their default orders to gather nearby ore and just stop beside the War Factory.

Placing the rally point closer (onto the building's bib) ensures that the target location is always a passable terrain.

Barracks' rally points are modified just for the consistency.

image

All in all, a minor QoL fix until #15995 gets properly fixed.

@ghost
Copy link

ghost commented Jun 16, 2019

I think the reason why the rally point is set with one cell between the exit is that otherwise the factory door stays open when you build a single unit. The position of the rally point is accurate to where in the original the unit would stop, so this could also be the reason why it was set up this way.

In any case the change is an improvement over the current behavior from a gameplay perspective, but we should fix the thing with the open factory door too.
If this was the indicator for "unit can't leave production facility" in the original then it should be implemented that way. I don't know if that was the case in the original because units scatter when they block an exit and even attack walls around the bib to make room.

oldra

@netnazgul You could also to do the same for the service depot in RA so its default rally point can not be blocked. there is not a bib but the rally point can be set anywhere on its footprint, since it is passable.

@WillowWideo
Copy link

If people find the door thing a big problem you can just change it to Offset: 0,2 instead
then the door closes just fine

@pchote
Copy link
Member

pchote commented Jun 19, 2019

I think the reason why the rally point is set with one cell between the exit is that otherwise the factory door stays open when you build a single unit.

This will need to be fixed, preferrably by making the door code smarter rather than putting the rallypoint to the side (which looks weird).

@abcdefg30
Copy link
Member

I thought the fix can be left for another PR. This PR only sets the rally point (still as seen in the image, I assume?). And the door animation was broken before already.

@reaperrr reaperrr mentioned this pull request Nov 17, 2019
@pchote
Copy link
Member

pchote commented Nov 17, 2019

If we want to go ahead with this idea IMO it would be better to implement the behavior from the original TS and later: hide the rally point indicator to reduce visual noise when it is set to the default exit, only showing it once it is explicitly changed by the player.

@netnazgul
Copy link
Contributor Author

@pchote your suggestion doesn't fix #15995 either unless you suggest to do both this and your suggestion.

@netnazgul
Copy link
Contributor Author

I've tried to address the WEAP flapping door issue, but couldn't make it work properly (close the door once the produced actor is out but still on the adjacent cell to it). Had a couple of approaches to checking the movement of a produced actor, and there is no clear way to know when the actor reaches the cell.

So, unless anyone else wants to give this a try, I suppose this PR should be closed if it is not to be accepted as is.

@tovl
Copy link
Contributor

tovl commented Nov 19, 2019

Note that #17021 already includes a fix for the underlying issue in #15995. However, I think defaulting to a closer rallypoint and not showing the flag when in the default position is still a nice polish idea. (I would assume that the suggestion by pchote is in addition to yours, otherwise it wouldn't make much sense—even with #15995 solved by other means.)

I would agree though, that the flapping door issue would need to be fixed first.

@pchote
Copy link
Member

pchote commented Nov 19, 2019

Indeed - my wording there could have been improved. I meant that by default we could not have an explicit rallypoint at all, meaning that units would stop at the exit cell until nudged out of the way like in the original game.

@tovl
Copy link
Contributor

tovl commented Nov 19, 2019

Yup, that is what I gathered.

Under the hood, it would be more convenient to have the default rally point be equal to the exit cell and not showing the indicators than to not have any rally point at all. This way, the behaviour would still be correct for 'Hot' bibs in Tiberian Sun (as introduced in #17141). The fix introduced in #17021 (using evaluateNearestMovableCell when (attack-)moving towards the rallypoint) would then make sure that the unit will move to the closest free cell that is not part of the bib.

By the way: If we do this, I think it would also be nice if there was a way for players to explicitly reset the rallypoint.

@tovl
Copy link
Contributor

tovl commented Dec 30, 2019

Should we close this and open up a new issue then?

@pchote
Copy link
Member

pchote commented Jan 26, 2020

Yes, but I jumped straight to a PR (#17625) instead.

@pchote pchote closed this Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants