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

Add chemical/magical fires #16676

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

FlameArrow57
Copy link
Contributor

@FlameArrow57 FlameArrow57 commented Oct 28, 2023

[GAME OBJECTS][FEATURE]

About the PR

This PR adds a new version of fire, chemical fire. Chemical fires are a resprited fire type intended to represent a fire created from a chemical or magical source, like wizard fireballs, that comes off the ground.

-They act the same as atmospheric fire and can be on the same tile.
-They have two layered parts to them, one that appears below objects on their turf, and one that appears over.
-Chemical fire color can be something other than red, dependent on the source. Ex. magnesium fires will burn white, potassium fires will burn purple.

This PR replaces all non-atmospheric fires with it

Also makes it so that the Wizard fireball spell will now leave fire behind that is the color of the Wizard's hat.

Note-
I realize that this could tie in more closely with chemicals. It was suggested on Discord these fires could relate to chemical reagents much more closely, such as duration dependent on chemical volume on the source turf, reactants available and their types (oxidizer, etc), and resultant fire temperature in relation to chemicals. However, I think that would really be a larger rework/rebalance that this PR wasn't really aimed at initially. This PR was mainly to focus on the visual part of fires created through /proc/fireflash().

image

image

image

image

image

image

image

Why's this needed?

All fires in the game are represented as atmospheric fires when some are really a chemical fire, so more accurate in that sense.

Adds a new 3/4s perspective version of fire.

Allows for colored fire

Changelog

(u)FlameArrow57, Flaborized, and Azrun
(*)Added a new type of fire, chemical fire, with sprites by Flaborized! This is a type of fire that acts the same as atmospheric fire but comes from chemical/magical sources, colored depending on the source, and can be on the same tile as atmospheric fire. Replaces all non-atmospheric fires.
(+)Fire created from the Wizard fireball spell will now be the color of the hat the Wizard is wearing.

@boring-cyborg boring-cyborg bot added the C-Sprites Automatically applied on any .dmi or icons folder change label Oct 28, 2023
@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 28, 2023
@keywordlabeler keywordlabeler bot added A-Game-Objects The point of this PR is to deal with a specific game object C-Feature A new feature or enhancements to existing features labels Oct 28, 2023
@FlameArrow57 FlameArrow57 removed the request for review from mordent-goonstation October 28, 2023 07:39
@M-Earthfire
Copy link
Contributor

NOTE: PR'd for the moment just to get thoughts/approval on the idea. Still some code to test more and needs improvement on sprites.

Wouldn't make it more sense to turn this into a draft then?

Personally, i like the idea.

@FlameArrow57
Copy link
Contributor Author

Sure

@FlameArrow57 FlameArrow57 marked this pull request as draft October 28, 2023 07:57
@PrimeNumb
Copy link
Contributor

Could you add screenshots of the new assets?

@FlameArrow57
Copy link
Contributor Author

Sure, they're just placeholders for the moment though

@colossusqw
Copy link
Contributor

Wait, does that mean we can get burning oil spills and horrible napalm leakages now? If so, fantastic.

@FlameArrow57
Copy link
Contributor Author

Yup

Copy link
Member

@Tarmunora Tarmunora left a comment

Choose a reason for hiding this comment

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

Not sure how this is going to play out - feels weird to have a second separate implementation of fire 🤷

code/modules/status_system/statusEffects.dm Outdated Show resolved Hide resolved
code/obj/chem_fire.dm Outdated Show resolved Hide resolved
code/modules/antagonists/wizard/abilities/phaseshift.dm Outdated Show resolved Hide resolved
@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 28, 2023
@CheffieGithub
Copy link
Contributor

I like them for the visuals since hotspots look strange not as part of atmos

@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Nov 3, 2023
@Flaborized
Copy link
Contributor

General consensus in dev channels is that the current sprite just doesn't quite work out - something better tiled (and perhaps a bit less busy) will be needed Otherwise, I think this is pretty much good to go

Makes sense! Ultimately it will be up to @Flaborized, or anyone else, if they want to sprite something more suited for this. Personally, I just do not have the spriting ability needed for this, and at least for connected sprites across tiles, a number more icon states would need to be sprited (15 at least, using blob tiles per reference, plus possibly animated states). Once sprited though, I'm willing to make any code needed to support

I also said this in the devchat discussion about the sprites but I don't have enough time/energy to draw up that many fire sprites :sheltersweat: . If someone else wants to draw all that then that's fine, or if not then if other devs would in general be ok just using the old sprites that'd also be fine by me (I'm not married to this particular sprite or anything), but I can't personally do that sorry!

@Azrun
Copy link
Contributor

Azrun commented May 8, 2024

ChemFire_Demo.mp4

What if something like this? Flames merged horizontally, additional "layer" added for vertical neighbor. Alpha mask to improve visibility of on-turf items.

@Tarmunora Tarmunora removed the E-DNM [Dev Only] Do Not Merge - can only be removed by the applier or with their explicit permission. label May 10, 2024
@Azrun
Copy link
Contributor

Azrun commented May 10, 2024

image

@FlameArrow57
Copy link
Contributor Author

ChemFire_Demo.mp4
What if something like this? Flames merged horizontally, additional "layer" added for vertical neighbor. Alpha mask to improve visibility of on-turf items.

Looks really nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Game-Objects The point of this PR is to deal with a specific game object C-Feature A new feature or enhancements to existing features C-Sprites Automatically applied on any .dmi or icons folder change E-Certified-Organic Will not be marked stale. S-Testmerged [Dev Only] Testmerged for extended testing (applied by bot) size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet