fairyglade/ly
13
605
Fork
You've already forked ly
65

Fix Doom fire animation; add flame height control #792

Merged
AnErrupTion merged 4 commits from asterane/ly:doom-fire-refresh into master 2025年07月08日 10:26:39 +02:00
Contributor
Copy link

I re-implemented the fire animation based on https://fabiensanglard.net/doom_fire_psx/, as it was done in the early days of Ly.

I added an option for natural fire colors, as even in text mode about five different colors are used for the best effect.

I added a fire height configuration; its setting changes how likely fire particles are to cool as they propagate upward.

This merge request fixes issue #784

I re-implemented the fire animation based on [https://fabiensanglard.net/doom_fire_psx/](https://fabiensanglard.net/doom_fire_psx/), as it was done in the early days of Ly. I added an option for natural fire colors, as even in text mode about five different colors are used for the best effect. I added a fire height configuration; its setting changes how likely fire particles are to cool as they propagate upward. This merge request fixes issue #784
AnErrupTion requested changes 2025年07月07日 12:18:41 +02:00
Dismissed
AnErrupTion left a comment
Copy link

Hey, sorry for the late review! Just a few minor changes needed and questions.

Hey, sorry for the late review! Just a few minor changes needed and questions.
res/config.ini Outdated
@ -111,0 +112,4 @@
# DOOM animation use natural fire colors
# If false, below custom colors used
doom_default_colors = true

Why not just set the default colors in the config to those natural colors?

Why not just set the default colors in the config to those natural colors?
asterane marked this conversation as resolved
@ -39,0 +55,4 @@
.allocator=allocator,
.terminal_buffer=terminal_buffer,
.buffer=buffer,
.height=@min(9,fire_height),

Please make this value as a constant, like STEPS.

Please make this value as a constant, like `STEPS`.
asterane marked this conversation as resolved
@ -65,2 +84,2 @@
constpropagate=self.terminal_buffer.random.int(u1);
constto=from-self.terminal_buffer.width;// Get the line above
// Generate random datum for fire propagation
constrandom=(self.terminal_buffer.random.int(u8)%10);

You can use intRangeAtMost() instead of using modulo here.

You can use `intRangeAtMost()` instead of using modulo here.
asterane marked this conversation as resolved
@ -67,2 +86,3 @@
self.buffer[to]=if(cell_index>0)cell_index-propagateelsecell_index;
// Select semi-random target cell
constto=from-|self.terminal_buffer.width-|(random&3)+1;

Same here. Also, I'm not sure if re-using the same random integer is a good idea.

Same here. Also, I'm not sure if re-using the same random integer is a good idea.
asterane marked this conversation as resolved
@ -73,0 +95,4 @@
// Choose new fire level and store in level buffer
varlevel_buf_to=level_buf_from;
if(random>=self.height)level_buf_to-|=1;
self.buffer[to]=@intCast(level_buf_to);

Is there actually a need to cast here?

Is there actually a need to cast here?
asterane marked this conversation as resolved
Author
Contributor
Copy link

Thanks for the review! Good points, I believe I addressed all of them.

Along with the height parameter to modify energy loss as the fire rises, I also added a spread parameter to modify how far energy can transfer between columns.

Thanks for the review! Good points, I believe I addressed all of them. Along with the height parameter to modify energy loss as the fire rises, I also added a spread parameter to modify how far energy can transfer between columns.
AnErrupTion left a comment
Copy link

LGTM. Thanks for your contribution!

LGTM. Thanks for your contribution!
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
fairyglade/ly!792
Reference in a new issue
fairyglade/ly
No description provided.
Delete branch "asterane/ly:doom-fire-refresh"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?