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

Overhaul event mob selection #13710

Merged

Conversation

dearmochi
Copy link
Contributor

@dearmochi dearmochi commented Jun 27, 2020

To-do List

  • Subsystem that handles polling for candidates from creation to end
  • Poll stacking e.g multiple blob infested mice available to control would only show one alert rather than multiple
  • Alert UI
  • Convert all /proc/pollCandidates calls to new subsystem proc
  • Port /proc/pollCandidatesWithVeto

What Does This PR Do

  • Creates the ghost_spawns subsystem which serves as the subsystem for polling candidates for a special role. On the outside it works the same as /proc/pollCandidates but unifies everything so that event mob candidacies are a consistent experience.
  • When a candidate poll is created through SSghost_spawns.poll_candidates, an alert is shown in the top-right corner of the screen to all ghosts with a timer. Clicking that alert signs the player up as a candidate

Why It's Good For The Game

  • Signing up for an event mob (blob, abductors, ERT...) is more consistent and doesn't require the player to interact with elements that vary in location. Everything is done through the top-right corner alerts.
  • Alerts display a timer as well as the number of open positions for a specific role, making it more obvious how long a poll lasts

Images of changes

image -> image -> image
Alert gets a white outline when the player signs up for the poll. Time text turns red when there are less than 10 seconds left

ezgif-6-2b993fcdc2d2
Poll stacking in action - polls stack if they have the same question and role. Clicking an alert with stacked polls signs the player up for all of them

Changelog

🆑
tweak: Overhaul event mob selection to make it more consistent, more intuitive and less frustrating
/:cl:

Create the SSghost_spawns subsystem which serves as the subsystem for polling candidates for a special role. On the outside it works the same as /proc/pollCandidates but unifies everything so that event mob candidacies are a consistent experience
Copy link
Contributor

@Kyep Kyep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objection.
Converting all "first come first served" ghost roles (e.g. terrors) to polling-based is not acceptable.

@TDSSS
Copy link
Contributor

TDSSS commented Jun 27, 2020

IIRC the reason some roles are first come first serve is because they quickly need a controller. Like if a terror spider spawns and doesn't do anything for 30 seconds because the poll is running, it might very well die before a player is picked. Does your system address that?

@SteelSlayer
Copy link
Member

SteelSlayer commented Jun 27, 2020

That can be changed with a bit more code, unless its simply not wanted at all that terrors be polled. For example, poll ghosts X number of seconds before a spiderling is about to turn into a terror, and then the ghost can be put in control of the spider instantly.

@dearmochi
Copy link
Contributor Author

dearmochi commented Jun 27, 2020

My main pet peeve with terror spider selection was that the only way (if I remember correctly) to take control is to click on a link in the chat. The problem with that is that you can easily miss the line and by the time you click, someone would have already clicked.
My plan was to tie the player selection to use the new subsystem which makes all polls use the same UI element (an alert) at a consistent location (top-right corner of viewport).
The actual selection mechanic can be further discussed but @SteelSlayer suggested a great solution to that problem IMO - poll for candidates right before the eggs hatch rather than when they hatch. Unless playing a terror should be first come first serve by design, I think that's the best route we could go for

@Kyep
Copy link
Contributor

Kyep commented Jun 28, 2020

Steelslayer:
The method by which terrors offer control to ghosts should not be changed.
It is carefully designed to do several things:

  • To allow people who are watching the spiderling/nest to take control of the spider before it is offered to the general ghost population. This ensures that the people who are most active/interested in a spider have a better chance of getting it. IE it rewards people who pay the most attention.
  • To ensure the spider gets a player controlling it quickly, and thus is less likely to die because an AI controlled it for 20 seconds in a busy area
  • To avoid timing issues such as what could happen if multiple spiders were offered to ghosts at once, and the same player selected "yes" for both, then won both rolls. Which spider do they end up in? what happens to the other one? My system avoids issues like this because there is no "polling" time - inhabiting a spider is instant.
  • To avoid spamming ghosts with tons and tons of popups they must individually select "yes" or "no" for (my notifications automatically vanish after a short time)
  • To allow for multiple spiders that grow up at once to present a menu of options and ghosts can choose one (by clicking the notice for the spider they want).

dearmochi:
The chat link for taking control of a spider is not the only way, or even the main way. The main way is the big notice you get at top right of the screen with a picture of the spider. You simply click that. The chat link is just a backup for people who are only reading chat and ignoring the game visuals.
Playing a terror should always be first-come, first-serve. This may also apply to some other types of event mobs which have similar properties to terrors. Would have to consider each event mob on a case-by-case basis.

@TDSSS
Copy link
Contributor

TDSSS commented Jun 28, 2020

I feel like we've had this discussion several times, but I'm still not convinced Terror Spiders should be first come first serve as long as the issues with polling can be solved. It is ultimately a very unfair system.

@McRamon
Copy link
Contributor

McRamon commented Jun 28, 2020

With terror spiders polling may happen for example 30 seconds before spiderling transforms (watching spiderling instead of game just to play it is weird design)

@AffectedArc07
Copy link
Member

AffectedArc07 commented Jun 28, 2020

You can make proper tick boxes in your PR description

  • Finish my changes
  • Push my commits to GitHub
  • Open a pull request

- [x] Finish my changes
- [ ] Push my commits to GitHub
- [ ] Open a pull request

@dearmochi
Copy link
Contributor Author

@Kyep

Ultimately I consider the goal of this PR to streamline (and improve) the polling process both visually and internally. However I'd like to address the points you've made to @SteelSlayer :

  1. Everyone has different reaction times, different PC specs and generally factors that may lead to them not being mechanically able to assume control of a spider in time. I'm talking about a <1s time frame to find AND click the right button. I think it's bad design to make observers constantly be on the lookout for a new spawn when they could be watching the game or chatting with other observers. This only leads to more frustration when they end up not getting a role they'd be interested in.
    Additionally, there's the concern of role hogging. Someone may constantly be the fastest to click thus getting the role but also the quickest to die. Back to observing they go and the cycle continues, denying other players of the role, which may have fared better if they'd got it.

  2. This point is moot if we poll before the egg hatches.

  3. Not possible with the new subsystem as candidacies are trimmed upon completion, removing mobs deemed invalid.

  4. The new subsystem allows for poll stacking. This means only one alert shows up for possibly multiple polls for the same role. Sign up for one role and any subsequent polls for an identical role will sign you up automatically.

  5. This is a valid point. Poll stacking means you can't choose (as it is) the specific poll you can sign up for, but that problem can easily be fixed by adding the choice of displaying buttons for individual polls next to the main alert. Hovering one such button would show the spawn location and clicking would sign the player up for that poll specifically.

Copy link
Member

@farie82 farie82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some first initial stuff. Haven't checked the logic much yet

code/_onclick/hud/alert.dm Outdated Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
@Fox-McCloud Fox-McCloud added Refactor This PR will clean up the code but have the same ingame outcome Work In Progress This PR is work in progress, and unfinished labels Jun 28, 2020
Copy link
Member

@AffectedArc07 AffectedArc07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few very minor thing

code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
code/controllers/subsystem/ghost_spawns.dm Show resolved Hide resolved
code/modules/events/blob.dm Show resolved Hide resolved
@dearmochi
Copy link
Contributor Author

dearmochi commented Jul 13, 2020

@Kyep I'd like to (re-)direct your attention to the comment I've written in response to the points you've made to SteelSlayer. I believe that removing the first-come first-served basis on terror spider assignment would alleviate a lot of frustration that ghosts may have during a terror spider event for virtually no downsides. One could argue that the RNG element of polling is a negative, but it's precisely intended to make selection fairer and not just restrict it to the fastest ones.

I haven't made anything related to that change as of yet - terror spider selection still operates the way it did before. All I would like is a definite yes or no from you based on my arguments before deciding if it should be part of the PR's scope. If it's a no, I'll remove it from the scope and submit my PR for review

@dearmochi dearmochi marked this pull request as ready for review July 14, 2020 19:46
@ZomgPonies
Copy link
Contributor

ZomgPonies commented Jul 17, 2020

I know I had a voicechat discussion with Kyep regarding terror spiders. Pretty much I think we agreed that as long as (astute) players have a couple of second to manually click on a new spider to get into it before the icon for the polling shows up, and as long as the polling differentiates between the different spider types then it should be okay.

Total poll time SHOULD be adjustable on a per type basis also. For example, spiders should have a low (10 seconds?) poll time, while others like ERT should have a longer one like 30sec-2min.

@Kyep - Feel free to correct me, it was pretty late when we talked.

Copy link
Contributor

@Kyep Kyep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved - so long as it doesn't affect terrors (or incorrectly say that it does).
There are some useful tweaks here, like showing the timer, that make the UI nicer to use.

code/controllers/subsystem/ghost_spawns.dm Outdated Show resolved Hide resolved
code/game/objects/effects/spiders.dm Outdated Show resolved Hide resolved
code/modules/admin/verbs/gimmick_team.dm Outdated Show resolved Hide resolved
code/modules/events/traders.dm Outdated Show resolved Hide resolved
@Daylight2
Copy link
Contributor

I know I'm a bit late to the party, but I'd like it if the new mob selection system played a sound as well, so alt-tabbed ghosts would have some other indication that roles were available other than the byond icon flashing.
The lack of sound was fine with the earlier first come first serve one notification per mob system, but if timed requests are going to use it, I'd prefer an audio queue too. (although roles that still use individual pop-ups per mob, like Kyet demanded for terrors, should probably refrain from spamming you with sounds. And while we're at it, could the sound not be the same as the station announcement sound? Thanks.)

code/_onclick/hud/alert.dm Outdated Show resolved Hide resolved
code/_onclick/hud/alert.dm Outdated Show resolved Hide resolved
@AffectedArc07
Copy link
Member

One the requested changes are integrated, this can be merged.

@AffectedArc07
Copy link
Member

Package repositories are dead, travis failing is not your fault. Remind me in 1 hour to re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor This PR will clean up the code but have the same ingame outcome Work In Progress This PR is work in progress, and unfinished
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants