-
Notifications
You must be signed in to change notification settings - Fork 269
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
fix: Molotov Fix/Rework, make turn_per_charge
less inconsistent.
#5480
Conversation
Fixes molotovs burning out too quickly due to my changes to charge handling. Makes it so that active items will now keep track of the turns they've been active if they use turns_per_charge. Molotovs now consume their own charges and burn out if left alone too long, they are reloaded with rags, one charge of which lasts about 5 turns.
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.
- Compiled and load-tested.
- Spawned in and activated a molotov.
- Tossed it, it go fwoosh yaaay.
- Spawned in rags and tested to make sure that changing them to ammo doesn't screw up their use actions.
- Crafted a molotov. It still consumes a rag as a component in the recipe, but produces a molotov with 0/1 ammo in it so you need to waste a second rag loading it.
4a191f1
to
d1dfaab
Compare
Looks like setting it to use tangible ammo makes it not be created with initial charges, will remove rags from the recipe, as I don't see any way to make recipes force the initial charge. |
Setting it to use rags as ammo makes it not spawn with default charges.
Specifying |
It already has it to begin with. It just doesn't apply outside of debug spawning it it seems. |
Ah right, damn. Guess that works but it feels real hacky having to load the rag manually after filling it, hmm. This is technically only mandated by it having |
It's technically mandated by me making them reloadable with rags. If we went by the old behaviour, every time it went out it would refill it's charges and you could light it again. |
Fair, I was wondering whether that'd technically be okay on the basis of the rag serving as a wick, but this would technically make the fuel infinite-use instead. This would only really matter if the player repeatedly lights a molotov and lets it burn out again and again until they start questioning why it hasn't run out of fuel by then, but that'd be a bit of a wacky scenario to begin with. |
To be fair another way we could do it is have it never burn out and only be "extinguished" by rain and water |
I think it just burning out after a bit but being able to be re-lit without having to provide a fresh rag is probably acceptable, compared to having to manually stuff the rag in before use each time after making it. Like I said, it would only look weird if the player lights and extinguishes it multiple times, whereas the rag thing will look weird every time one's crafted. |
I feel like the latter makes more sense than the former, though like... we could also just make it burn forever and have the activation effect allow you to manually extinguish it. I just feel that the behaviour where you can wait it out and relight it is very odd, and codewise looks kind of weird. |
That would work I guess too? I just find it really weird having to craft a molotov and then basically "fuze" it manually afterward. It stands out as kinda weird when every other grenade-type item in the game is crafted as an all-in-one deal. |
Okay. I also recalled that |
maybe we could make molotov use oil as charge? |
Pretty sure it having an ammo type at all is what's causing it to spawn with no charges, so changing that won't change the problem. |
oh, i meant as a balance change. |
Oh, I see now. Fair enough in that case, we can keep that for now and add an option to manually extinguish it in a later PR then. |
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.
These should be all the changes needed to make it do the "return to a lightable molotov if it goes out" thing, lemme know if I missed anything.
There's still |
Ah, no worries then. I'm currently out and about on wifi so won't be able to compile-test the new changes until I get home, so take your time if needed. ^^ |
So to be fair, I only tested spawning it, not crafting it. Why it spawns as 0/1 when crafted is beyond me. |
Because it's been given an ammo type, there's no code to make it spawn with specific ammo and count, and it defaults against that in case you're making magazines and the like. Anyway, code's rewritten, updating the original PR. I'm annoyed, but hey, it works about right now |
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.
Hmm. I thought the idea was they'd burn out and revert back to an unlit molotov if you don't throw them quickly enough? |
All three implementations have issues, some are under the hood, and some are overt. Original (intended) implementation
Rag implementation
New implementation
I don't like the first implementation because it's a lot of special case code handling for 1 specific iuse that needs to be careful about not treading on anything else's toes. And I can't just revert it to the original implementation and expect it to be fine because the original code as is doesn't revert at all when Putting this in draft while I think of a better stopgap that compromises between them and is acceptable to all of us. |
Base molotov has no charges, instead, it transforms into a lit molotov that has charges, that over time depletes it's charges and turns back. Disallow extinguishing it manually, as this produces molotovs with no max charge but a charge. They don't stack in the UI with other molotovs This issue will still occur if it gets extinguished by water.
b9f34a2
to
d09b9c3
Compare
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 mean, It works as intended and works under our old system of being relightable forever. It's not ideal but it's what we have right now.
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.
Hopefully is fine then, still out and about so can't compile-test weh
|
Checklist
Required
main
so it won't cause conflict when updatingmain
branch later.Purpose of change
turns_per_charge
less inconsistent by having tool items keep track of how long they've been active since last consumption.Describe the solution
Molotov behaviour is now changed in the following ways
Tool items with
turns_per_charge
now keep track of how long they have been active viaturns_active
variable.turns_active
exceedsturns_per_charge
the item consumes it's per use charges and resetsturns_active
.Describe alternatives you've considered
Testing
Additional context