-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Teleport noobs off Arrivals shuttle to spawn #17189
Teleport noobs off Arrivals shuttle to spawn #17189
Conversation
- Estimate arrival time
@metalgearsloth This Arrivals PR transports players off the arrival shuttle. It spaces them if they come back to the shuttle. Should it teleport them every time? |
Nah just the first time, we just need turnstiles for other times to avoid people using it for easy spacing #16419 |
Nice to see you in the code owner rotation now @EmoGarbage404 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very cool, just a few things that caught my eye.
// Spawn them at a passenger location | ||
foreach (var (spawnPoint, xform) in points) | ||
{ | ||
if (spawnPoint?.Job?.ID != "Passenger" || _station.GetOwningStation(spawnPoint.Owner, xform) != stationId) | ||
continue; | ||
|
||
// Move the player to the spawnpoint. | ||
transform.Coordinates = xform.Coordinates; | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All stations have latejoin spawn points already so there isn't really a need to have a hardcoded backup to passengers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any harm in doing so though? I'm coding defensively here I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's unnecessary but moreover, hardcoding in a prototype ID is generally frowned upon.
Thanks for the review @EmoGarbage404. I have applied your suggestions which was mostly changing how I got TransformComponent and adding better comments. |
5f502df
to
04d86c1
Compare
|
||
// The ArrivalsCooldown includes the trip there, so we only need to add the time taken for | ||
// the trip back. | ||
comp.NextArrivalsTime = _ticker.RoundDuration() + | ||
TimeSpan.FromSeconds(_cfgManager.GetCVar(CCVars.ArrivalsCooldown) + tripTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be using RoundDuration and also += like NextTransfer is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed roundDuration. However it doesn't make sense to use += here because the time estimate is not based on the old time estimate.
var possiblePositions = new List<EntityCoordinates>(); | ||
var passengerPositions = new List<EntityCoordinates>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValueList?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, I didn't know we had that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now require space-wizards/RobustToolbox#4257 for Pick(ValueList)
About the PR
Requires: space-wizards/RobustToolbox#4257
Media
arrival_teleport.mp4
Changelog
🆑