added particle system frontend #194
Conversation
There was a problem hiding this comment.
I like the idea of including particles in SplashKit as they could greatly enhance the visuals or gameplay of a game. In this PR there are some formatting issues to resolve as well as the need for more comprehensive documentation of the methods. The PR documentation should also be fleshed out.
I think it would help greatly to include a simple demonstration of the particle system working. It could be added to test_graphics.cpp or a similar file. That way it would be much easier to appreciate how it works.
|
|
||
| #define MAX_PARTICLES 1000 // Maximum number of particles | ||
| #define STREAK_HISTORY 20 // Number of positions to track for the streak | ||
| #define PI 3.14 // setting the PI value |
There was a problem hiding this comment.
M_PI is defined in cmath so no need to redefine.
| #define PI 3.14 // setting the PI value | ||
|
|
||
| // Particle structure to represent individual particle properties | ||
| struct Particle |
There was a problem hiding this comment.
Structs normally use snake_case naming in SK.
| }; | ||
|
|
||
| // Particle system to manage all particles | ||
| struct ParticleSystem |
There was a problem hiding this comment.
Snake case would be best here too.
| bool glow_enabled; // Whether this particle glows | ||
| float weight; // Particle weight | ||
|
|
||
|
|
| bool gravity_enabled; // Toggle for gravity | ||
| int emission_rate; // Multiplier for particles emitted per frame | ||
|
|
||
|
|
|
|
||
| }; | ||
|
|
||
| // Initialize the particle system |
There was a problem hiding this comment.
All of the methods below will need more comprehensive documentation. See some of the other header files for reference of the required format.
structure.txt
Outdated
There was a problem hiding this comment.
Is this auto-generated? Would be best to not include this in the PR if its not required.
| // Particle system to manage all particles | ||
| struct ParticleSystem | ||
| { | ||
| Particle particles[MAX_PARTICLES]; // Array of particles |
There was a problem hiding this comment.
Is there always going to be the maximum number of particles? If not, std::vector could save some memory.
|
Oh and I just realized this PR is to the main splashkit-core repository, when it should be against the Thoth Tech fork: https://github.com/thoth-tech/splashkit-core |
| #include <cmath> | ||
|
|
||
| // Initialize the particle system | ||
| void initialize_particle_system(ParticleSystem &system, double x, double y) |
There was a problem hiding this comment.
Initialize_particle_system takes double x, double y parameters which is used to set emitter_x/y and last_x/y which are floats.
Should follow SplashKit's dynamic allocation pattern
Need to set pointer identifier on creation
particle_system create_particle_system(double x, double y)
{
particle_system result = new _particle_system_data();
result->id = PARTICLE_SYSTEM_PTR;
result->emitter_x = x;
result->emitter_y = y;
// ...
return result;
}
Add Freeing Function:
void free_particle_system(particle_system to_free)
{
if (VALID_PTR(to_free, PARTICLE_SYSTEM_PTR))
{
to_free->id = NONE_PTR;
delete(to_free);
}
}
All functions that use the particle system would need to validate the pointer:
if (!VALID_PTR(ps, PARTICLE_SYSTEM_PTR))
{
LOG(WARNING) << "Invalid particle system";
return;
}
| }; | ||
|
|
||
| // Particle system to manage all particles | ||
| struct ParticleSystem |
There was a problem hiding this comment.
The particle system should use a pointer-based approach because SplashKit uses an FFI (Foreign Function Interface) to expose its functionality to other languages. Direct struct passing across the FFI boundary can be problematic due to:
Different memory layouts between languages
Different padding/alignment rules
Difficulty tracking object lifetime across language boundaries
This is why SplashKit consistently uses pointer types (like animation, sound_effect, timers, etc.) rather than passing structs directly.
Internal data structures should use snake_case with leading underscore and should add the pointer identifier for validation
struct _particle_data { ... }
struct _particle_system_data {
pointer_identifier id; // For FFI pointer validation
// ...
}
typedef _particle_system_data *particle_system;
You can also consider using:
point_2d position; // point_2d instead of separate x,y
vector_2d velocity; // vector_2d instead of separate dx,dy
vector<_particle_data> particles; // Dynamic array for flexibility
point_2d emitter_pos; // point_2d
point_2d last_pos; // point_2d
vector<point_2d> streak_positions; // vector of point_2d
| float distance = sqrt(dx * dx + dy * dy); | ||
|
|
||
| // Skip segments with large distances (to prevent the straight line artifact) | ||
| if (distance > 50.0f) // Threshold for skipping outliers |
There was a problem hiding this comment.
50.0f may be better as a constant
There was a problem hiding this comment.
The core functionality is solid, but my suggested changes will help it integrate better with SplashKit's FFI requirements and type consistency.
|
It looks like you have inadvertently included a large number of files that seem to be unchanged. You will need to look at this if you want this merged at some point, as we cannot accept this as it is at the moment. |
Description
I implemented the frontend of the particle system using SplashKit functions. SDL backend is being developed at the moment.
Type of change
How Has This Been Tested?
Since the particle system files are separate, I tested it after building the library. All the functionalities I added worked successfully.
Checklist