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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper lobby art system #15850

Closed
wants to merge 1 commit into from
Closed

Conversation

XElectricX
Copy link
Contributor

@XElectricX XElectricX commented May 12, 2024

About The Pull Request

Was just going to be looping through different lobby art images but it necessitated a lot of changes.

  • Lobby art fades into the next image
  • How long the art stays up and how long the fading is can be changed via config (default 30 and 5 seconds respectively)
  • Split up each piece of art into it's own file; had to give them names so made some up (if you are the artist of one of these images or know the title, please comment it)
  • The system automatically looks for any images in the designated directory to pick an image to use (said directory can also be changed via the config)
  • This means any future artist can also PR a new image quite easily, just drop a new .dmi file in the directory (the sprite name MUST be blank)

A map edit was made since the lobby art was actually a turf, which caused issues (and is a dumb way to handle it). The rest of the edits were StrongDMM doing it's sanitization. This PR requires #15848 to work, otherwise the players won't see anything.

Why It's Good For The Game

We love lobby art. Let us see the rest.

Changelog

馃啈
imageadd: Split the lobby art file into their own individual files.
imagedel: Deleted old lobby art file.
refactor: Lobby art system exists now instead of just being an icon_state assignment on a turf.
config: Lobby art directory, duration, and fade time can be set in config.
/:cl:

@tgstation-server tgstation-server added Sprites Changes to .dmi file. Config Update Changes to server configuration options. Refactor Improves underlying code to make systems more modular and functional. Map Edit One or more changes to .dmm files. labels May 12, 2024
@XElectricX
Copy link
Contributor Author

I think the icon bot might have broken from checking too many files.

Comment on lines +2 to +3

/obj/effect/lobby_art
Copy link
Member

Choose a reason for hiding this comment

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

?? why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why what? effect types are good for abstract things that shouldn't be interacted with.

Copy link
Member

Choose a reason for hiding this comment

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

It was a turf earlier though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And now it's not.

Comment on lines +27 to +28
layer -= 0.1 //I could use initial() but I don't see an instance to where anything should be changing the layer of it (minus this)
QDEL_IN(src, CONFIG_GET(number/lobby_art_fade_duration) SECONDS)
Copy link
Member

Choose a reason for hiding this comment

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

Im denying the fade, we already have animated lobby UIs that swap and I dislike how it looks, people should be able to look at one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is quite literally just your opinion. I think it looks great. The animated bit is also silly, they still play out completely within 30 seconds. I could also just isolate the animated ones so the art doesn't transition if an animated one is selected roundstart if it really bugs you.

TM it and if the majority of people hate it then I can just close it. 馃し

Copy link
Member

Choose a reason for hiding this comment

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

You're entitled to your opinion but I am not interested in allowing this to be merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who died and made you the aesthetics police???

@github-actions github-actions bot added the Merge Conflict Pull request is in a conflicted state with base branch. label May 15, 2024
@TiviPlus
Copy link
Member

If you dont wanna change it ok then

@TiviPlus TiviPlus closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Config Update Changes to server configuration options. Map Edit One or more changes to .dmm files. Merge Conflict Pull request is in a conflicted state with base branch. Refactor Improves underlying code to make systems more modular and functional. Sprites Changes to .dmi file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants