Yet some more fixes #170
Yet some more fixes #170
Conversation
pattakosn
commented
Feb 17, 2025
•
pattakosn
commented
- forgotten redundant ifdef
- capitalization issues (linux is case sensitive)
- header path fixes
- replace nullptr with VK_NULL_HANDLE
- unused variables
- rename local variables that are hiding outer scope variables
- passing redundant function parameters of variables that are already in the namespace
pattakosn
force-pushed
the
yet-some-fixes
branch
from
08594f9 to
948c458
Compare
build_scripts/premake.lua
Outdated
There was a problem hiding this comment.
Is there a reason for this? It deviates from the norm, which is third_party/name.
There was a problem hiding this comment.
the reason was that as it was, `#include <meshoptimizer/meshoptimizer.h> was needed which (AFAIK) is not the upstreamm way to include it and i couldnt use the meshoptimizer package i have on my linux system (installed with my package manager) the other solution is the one i just pushed: mv the header one dir up
There was a problem hiding this comment.
My understanding is that the package wants it this way, if that's the case then do what you think is best, we can live with one library deviating a bit for now.
There was a problem hiding this comment.
removing the dots?
my understanding is that to get from runtime/Geometry to runtime/RHI/RHI_Vertex.h you need to go up one level.
This matches the behavior on my system(compile error).
I am really surprised and curious how it compiled on yours (i will test master on a windows machine tomorrow and confirm that)
There was a problem hiding this comment.
The pipeline of the other PR is failing because of this https://github.com/PanosK92/SpartanEngine/actions/runs/13402678342/job/37436651747?pr=171
|
Is there a reason for that library path change? |
pattakosn
force-pushed
the
yet-some-fixes
branch
3 times, most recently
from
7ad849c to
57e82d7
Compare
runtime/Game/Game.cpp
Outdated
| @@ -636,8 +636,8 @@ namespace spartan | |||
| renderable->SetInstances(instances); | |||
|
|
|||
| // make the bark collidable | |||
| shared_ptr<PhysicsBody> physics_body = bark->AddComponent<PhysicsBody>(); | |||
| physics_body->SetShapeType(PhysicsShape::Mesh); | |||
| shared_ptr<PhysicsBody> physics_body_bark = bark->AddComponent<PhysicsBody>(); | |||
There was a problem hiding this comment.
Could this be that you are mistakenly redeclaring this here and you are actually hiding the outer one?
Could it be that this physics_body is supposed to be the same as in line 576 for example?
There was a problem hiding this comment.
They will still affect the same memory but the double declaration is a mistake, you can remove physics_body_bark if you want.
pattakosn
force-pushed
the
yet-some-fixes
branch
4 times, most recently
from
9522a65 to
b46d5e3
Compare
pattakosn
force-pushed
the
yet-some-fixes
branch
from
b46d5e3 to
1ff1791
Compare
