Box versus hull within mMaxSeparationDistance sometimes results in flipped mPenetrationAxis

[mihe]

This is an issue that was reported in godot-jolt/godot-jolt#536, which seems somewhat sensitive to the various parameters and placement of things, as well as compiler flags like -ffast-math.

In short, if you do a sCollideShapeVsShape between a box and a convex hull, with both shapes using a convex radius of 0, but a mMaxSeparationDistance of 0.001, while the box is hovering above the hull but still within mMaxSeparationDistance, the penetration axis will sometimes be flipped depending on the placement and orientation of the two shapes.

I created a test for it in Samples, inspired by CapsuleVsBoxTest, which you can find in this fork/branch here.

Here's what the test looks like:

https://github.com/jrouwe/JoltPhysics/assets/4884246/b02a7274-255a-4bd8-ad1b-d6ebbf68eeae

The hull is the bottom box, and the actual box is the top one. They both rotate continuously, but the relative position/orientation between them is kept the same.

It should only ever draw a green line upwards, which is the expected normal (flipped mPenetrationAxis), but sometimes you'll note that it draws a red line downwards instead.

Like I mentioned, this seems very sensitive to what parameters you use, which I presume means it has something to do with floating-point precision or some such thing. Any change to the relative position/orientation of the two shapes yields a seemingly random outcome in terms of whether you'll reproduce the issue or not.

[jrouwe]

This one is going to take a lot of time to debug as the GJK/EPA algorithms are very sensitive to float precision errors, it will take me some time to get to this.

[jrouwe]

I did a preliminary investigation: The GJK algorithm detects the collision between the bodies, but the algorithm terminates early because it reaches float precision limits. This means it doesn't return a contact point and the EPA algorithm needs to run. The EPA algorithm builds a convex hull, but in this case the convex hull is practically a 2D rectangle because the overlap between the two shapes is so small (it's a bit complex to fully explain what EPA does). In any case, it does a number of iterations and then makes a mistake (due to a floating point precision issue) when adding a new point to the hull and the normal flips:

Side view of the hull shows it has no volume:

I will do some further investigations to see if the convex hull building algorithm can be made more tolerant, but it is incredibly complex to make this algorithm robust for any degenerate shape you throw at it.

In any case, there is a really easy way to make the whole thing a lot more robust. Right now the box and convex hull have a convex radius of 0, this means that the shape GJK sees is exactly the shape EPA sees. If you give the box even a tiny (0.001) convex radius, the problem in the demo disappears. This change means that GJK is able to detect collision points up to a greater penetration depth and when EPA runs, the overlap between the bodies is much bigger, meaning the float precision problems while building the hull are greatly reduced. Personally I would go for a convex radius of at least 0.01 for a shape this size. This could also help with the problem in #630 as the edges of the box will be rounded so you're less likely to get a contact normal pointing towards you.

@Gamepro5 is there a reason why you selected a convex radius of 0? (or is this only something in @mihe's example?). Note that the convex radius is called 'margin' in godot.

[mihe]

@Gamepro5 is there a reason why you selected a convex radius of 0? (or is this only something in @mihe's example?). Note that the convex radius is called 'margin' in godot.

This was present in the original repro as well, and not something that I added, although I suppose I am responsible for it being there.

Similar to the effect that EActiveEdgeMode::CollideOnlyWithActive has on collision normals (as mentioned in #630) it seemed that the rounded collision normals that appear as a result of a non-zero convex radius (as discussed in #628) had a detrimental effect on @Gamepro5's character controller, and as such the option to disable them globally (i.e. setting them all to 0) was added to Godot Jolt, and why it was set to 0 in the test provided above. Obviously I wasn't aware of edge-cases like this when I recommended that they be set to 0 though.

As a potentially related tangent, what is the relationship between the convex radius and mMaxSeparationDistance? I've seen the two get added together in places like sCollideConvexVsConvex, so I figured that maybe a mMaxSeparationDistance (like the one set in the test above) would effectively add a bit of convex radius to a shape that doesn't already have it, albeit as a "skin" rather than just rounding the edges, but since it doesn't seem to have the same side effects as a convex radius (nor resolve this particular issue) I'm guessing that's not how that ends up working?

[Gamepro5]

Indeed. The effect it had was that it would give 'rounded edges' to shapes and make them not report the correct normal when standing on the edge of a shape (like standing really close to falling off a cliff)

[jrouwe]

So I found what the issue is and I have submitted a fix for this particular issue, but since there are an infinite amount of permutations possible, it is impossible to say if this will actually fix all cases. The EPA algorithm is full of heuristics that try to detect and correct for floating point precision issues and these issues always take a lot of time to debug (a full day in this case). @mihe I turned your CapsuleVsBoxTest into a unit test to prevent regression. @Gamepro5 can you try this out (as soon as there's a Godot Jolt version that has it)?

[mihe]

I can confirm that the change fixes the repro project that @Gamepro5 provided.

Thank you!

[jrouwe]

Cool, thanks for testing!

[Gamepro5]

Absolutely. I'm cursed with finding bugs, so I will give it my best!

Thank you for the fix!

[Gamepro5]

@jrouwe so I found a lot of regressions when using this update on my character controller, but everytime I try to isolate it into an MRP the issue doesn't seem to be there. I know that I'm not going crazy though because downgrading to a 3 week old version of godot jolt does not have the same bugs appear in my character controller. It's just going to take a long time before I can isolate each one.

From what I noticed so far, though, it really doesn't like "use shape margins" to be disabled (I disabled that to remove the "rounded edges" that appear from the normal reporting). In these cases, the depenetration will be wrong and the shape will sometimes teleport to the underside of the shape and just hang there in a constant state of colliding with it.

In other words, it seems this update made collisions with trimesh shapes much worse when not using shape margins, which is odd because all it was supposed to address was that weird rotation thing.

[mihe]

I can only really confirm the surface-level symptoms right now, which is that 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e seems to have a mostly negative effect on this particular project (which is doing a lot more than the original repro) and that doing nothing but reverting 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e (or using a non-zero convex radius) seems to undo those negative effects.

As for the actual cause, or whether this has just surfaced some other problem, I can't really say, but someone will probably need to break this down to the actual collision check in order to find out.

[jrouwe]

Ok, too bad. I've reverted the change in 2fef04f0264cb74764caa354151964e56d11e18a. If you can somehow give me a repro case where it goes wrong with the new code then I can do some further investigations.

In any case, if I create the algorithm as described in #630 then I think we can use a convex radius because the algorithm should have a better idea of active vs non-active edges and report the triangle normal in most cases.

[Gamepro5]

I managed to narrow down one of the differences here. The other ones I described I cannot seem to isolate, sadly. My hope is that this discrepancy will illustrate the bigger issue that led to the one I was unable to reproduce. https://github.com/godot-jolt/godot-jolt/issues/536#issuecomment-1696400871

[mihe]

I looked into the two new repros. One of them seems to actually be an improvement as a result of 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e, where a collision between a BoxShape (with no convex radius) and a sizeable triangle in a MeshShape, that rightfully should be reported, went unreported prior to that change, and interestingly also unreported in Godot Physics. The other one is simply an expected change in behavior as a result of increasing mActiveEdgeCosThresholdAngle to 50 degrees.

So the actual concrete negative effects of 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e remains a bit of a mystery still.

[mihe]

One of them seems to actually be an improvement as a result of 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e, where a collision between a BoxShape (with no convex radius) and a sizeable triangle in a MeshShape, that rightfully should be reported, went unreported prior to that change, and interestingly also unreported in Godot Physics.

It seems I made a mistake when looking at this one. This might indeed be a regression caused by 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e, as the reported penetration depth between the box and the triangle(s) seems to have worsened in this particular case.

I'll try and distill it down to a Jolt repro.

[mihe]

I'm not sure what to make of this one.

Here is the repro, which sort of emulates the iterative depenetration that move_and_collide does in Godot.

The problem in that particular case is that with the default depenetration iterations of 4 and factor of 0.4 you end up with a remaining depth that's greater than mMaxSeparationDistance, which results in a discrepancy with Godot Physics, whereas it didn't before 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e. However, due to the nature of the depenetration factor, which if I remember correctly is meant to prevent jitter/ping-ponging when faced with multiple collisions, you're never really guaranteed to be fully outside of mMaxSeparationDistance anyway, so the actual problem here is somewhat arbitrary and self-inflicted.

Also, while this obviously behaves differently when applying 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e, and perhaps worse by some measure, it's clear when looking at the current state of things that this has never worked all that well when using a box with a convex radius of 0. It just so happens that it currently (i.e. without 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e) overcorrects pretty quickly, to the point that it ends up working out in this particular context.

The workaround here is to simply throw a couple of more iterations at it, or tweak the factor a little bit, which are called "Recovery Iterations" and "Recovery Amount" in the Godot Jolt project settings. Setting the iterations to something like 6 or above "resolves" this particular problem in the repro, as well as the original repro.

What it doesn't resolve however is the jank when climbing up the convex hulls (i.e. the stair-like objects) in this project. Without digging into that character controller there it seems like 5d382fc0f71411dacf0c1f6f93fe0d2612fed45e has introduced some other regression (or at least discrepancy with Godot Physics) with regards to collisions between BoxShape and box-shaped ConvexHullShape, when using no convex radius on either one, so I'm not convinced that the repro I provided here is the main villain.

[jrouwe]

Ok, thanks again for the repro. I'll see what I can make out of this.

[jrouwe]

I've made another attempt at fixing this: #690. It seems to work fine with both of @mihe's test cases (box vs convex hull reports the correct normal and box vs triangle reports the same values as without this change). I tried running Custom_CharacterBody3D_Movement_System, bug060.zip and bug.zip and I think they're ok but I'm not 100% sure what to look for. bug.zip traces nothing and bug060.zip traces [(0.186155, 0.694745, 0.694749), (0, 1, 0)].

If I compiled stuff correctly, then this is a Distribution and EditorDistribution build with the new code for x64:

godot-jolt.zip

@Gamepro5 perhaps you can try it out.

[Gamepro5]

@jrouwe It does indeed seem to me as though you fixed it. I'm curious, what did you change to fix it? How is this new approach that seems to work great different from the old one?

but I'm not 100% sure what to look for.

I noticed instability near edges of concave shapes with the older implementation. I tried moving along these edges a lot and it does seem like you fixed it, great job!

[mihe]

The jank when traversing the stairs made of convex hulls is gone now as well, so it seems those issues were related after all.

Nicely done!

[Gamepro5]

bug.zip

I seem to have found a bug with this one. I think I may have found others but I need to MRP them first.

Here's another: bug2.zip

Just to show that I'm not trying to nitpick, this is how it behaves on this specific shape. https://youtu.be/UwYtBa4wHn0

If there is anything I can do to help make my MRPs more minimal for your convenience, please let me know.

[mihe]

bug.zip

This looks to be similar-ish to the original repro here, in that it's likely some floating-point problem. Moving the character and the ramp closer to [0, 0, 0] makes it result in the same outcome as Godot Physics, where no collision is reported.

I'll try and distill this one down to a Jolt repro.

bug2.zip

This looks to be another instance of scenario 2 described here, so isn't related to this issue.

If there is anything I can do to help make my MRPs more minimal for your convenience, please let me know.

Removing anything that doesn't actually affect the repro would be appreciated, like anything but the ramp in that first one, which is quite a lot.

[Gamepro5]

Don't trust Godot physics on this shape. Those shapes are actually the reason I switched to jolt, because godot had too many bugs to count when colliding with these shapes.

That is to say, just because it reports no collision doesn't necessarily mean that godot physics is correct. It may be in this case, but not all cases. I would often fall though the level when using godot physics.

[jrouwe]

Alright, attempt nr. 3: #691. As far as I can see it works in all repro cases reported so far. Note that it only (possibly) fixes the flipped normals. Not getting caught on ledges as reported in #630, and I don't know how to run https://youtu.be/UwYtBa4wHn0 so I haven't looked at that either.

godot-jolt.zip

As before: I can't predict if this will fix all cases as floating point precision issues are incredibly hard to work with.

[mihe]

and I don't know how to run https://youtu.be/UwYtBa4wHn0 so I haven't looked at that either

I'm not sure if it's the exact same level as what's shown in @Gamepro5's YouTube video, or even the same controller, but something akin to that level is bundled with this project, so I replaced the dev_test node in game.tscn with maps/catwalk.tscn, moved the character's third-person camera a bit closer, enabled the physics debug drawing through "Visible Collision Shapes" in the editor's "Debug" menu, and here's what it looks like before #691:

https://github.com/jrouwe/JoltPhysics/assets/4884246/4e600ee7-5a88-435c-93ab-4665e2417ba3

It might be hard to tell, but there's plenty of jank and getting stuck, similar to the YouTube video.

Here's what it looks like after #691:

https://github.com/jrouwe/JoltPhysics/assets/4884246/5745722f-0a6c-4d4d-9c08-e55f25e18659

It's hard to tell whether there's any improvement. There's still plenty of jank and getting stuck, specifically when an edge/corner of the character collides with an edge/corner of these convex hull ramps, when "far" away from the world origin.

I decided to also test my theory of just upping the recovery/depenetration iterations from 4 to something like 8-10, since the misreported penetration depth seems to at least be part of the story here, and while that does seem to have a positive effect at "normal walking speeds", the jank still presents itself when moving slowly, for reasons I don't entirely understand.

How do you want to proceed? Would breaking this down into more repros help?

[jrouwe]

I don't know if the jank is due to the contact normal pointing towards the ground instead of away (because that's the only thing I've been looking at so far), that would be a good thing to know.

Other than that, I think the best way to proceed is to give the character a convex radius > 0 to eliminate this problem and to use the code I'm writing for #630 to correct the resulting contact normals.

[mihe]

I don't know if the jank is due to the contact normal pointing towards the ground instead of away (because that's the only thing I've been looking at so far), that would be a good thing to know.

I put a print in my implementation of move_and_collide if any downwards-facing normal was reported in any of the phases, and I don't see a single print when provoking the jank shown above, nor when just traversing the level in general, unless I jump up and hit the ceiling. This seems to be the case both with and without #691, so I'm not quite sure what's going on here.

Maybe @Gamepro5 can shed some light on what it is the controller is expecting vs actually receiving that's causing this.

[Gamepro5]

Maybe @Gamepro5 can shed some light on what it is the controller is expecting vs actually receiving that's causing this.

Yeah, I think it's time.

My character controller is a replacement for the move_and_slide() system that godot uses, because back when I wrote it, move_and_slide() was really bad for 3D. I don't know if it still is, but it was.

Here's what it does and how it pertains to getting accurate normals:

First, a move_and_collide test is done to see if there is something directly under us. This is the branch where we either decide to run the air movement code or the ground movement code. The ground movement code is what is relevant here.

In the ground movement, first I do a snap check. This is to make sure the player is snapped to the floor. The snap vector is sometimes set to Vector3.ZERO when the following script deduces that we are not on a floor and are instead on a steep slope.

var snap_ground_check = move_and_collide(snap_vector*0.001, true, 0.001, true, 5)
    if !snap_ground_check: #this prevents small false positives from adding up and making the player appear to slide down a slope even with zero velocity
        snap_ground_check = move_and_collide(snap_vector*snap_magnitude, false, 0.001, true, 5) 
    if snap_ground_check:

what comes after is storing the normals that are considered to be floor normals (using normal.angle_to(Vector3.UP) <= max_flr_ang_rad) and taking the average of all normals that are considered too steep and assessing that average normal. This is done so that the player can still treat being wedged in a V shape as a floor, despite both reported normals being too steep to be individually considered floors.

see diagram

Next, I do the last move_and_collide() that influences the movement calculations.

var col = move_and_collide(vel*delta, true, 0.001, true, 5)

This one is fairly simple, so I'll put the code here:

for i in range(col.get_collision_count()):
    var normal = col.get_normal(i)
    if (normal.angle_to(Vector3.UP) <= max_flr_ang_rad): #slope counts as the floor
        floor_collision_normals.push_back(normal)
    else: #collision is not the floor. it is either a ceiling or a wall.
        if rad_to_deg(col.get_angle(i, Vector3.UP)) > 91: #collision is ceiling
            ceiling_collision_normals.push_back(normal)
        else: #collision is wall
            wall_collision_normals.push_back(normal)
for i in ceiling_collision_normals:
    ceiling_collision_normal = i
    vel = ceiling_collision_solver(vel, delta)
for i in wall_collision_normals:
    wall_collision_normal = i
    var temp = vel
    vel = wall_collision_solver(vel, delta)
    if wall_collision_normal == Vector3.ZERO:
        break #a stair step was made.
for i in floor_collision_normals:
    floor_collision_normal = i
    vel = floor_collision_solver(vel, delta)

The purpose of this test is to test for walls, new slopes to slide on, and ceilings. This differs from the snap check because this moves in the planned direction of the player.

The individual solvers for each normal are as follows (with the wall one being simplified to exclude the quake stair stepping algorithm):

Floors:

func floor_collision_solver(_vel, delta):
    _vel.y = 0
    _vel.y = (-floor_collision_normal.z*_vel.z-floor_collision_normal.x*_vel.x)/floor_collision_normal.y
    if (floor_collision_normal.y == 0): #safeguard. if the y normal of the slope is 0, it means you are trying to climb a completely vertical wall, so the compensation to not affect x/z velocity is infinity. good luck with that.
        _vel.y = 0
    return _vel

ceilings:

func ceiling_collision_solver(_vel, delta):
    if (wall_collision_normal != Vector3.ZERO):
        var wall_floor_tangent = wall_collision_normal.cross(floor_collision_normal).normalized()
        _vel = wall_floor_tangent * _vel.dot(wall_floor_tangent)
        var ceiling_floor_tangent = ceiling_collision_normal.cross(floor_collision_normal).normalized()
        if wall_floor_tangent.dot(ceiling_floor_tangent) <= 0:
            _vel.x = 0
            _vel.z = 0
        else:
            _vel = ceiling_floor_tangent * _vel.dot(ceiling_floor_tangent)

        ceiling_floor_tangent = ceiling_collision_normal.cross(floor_collision_normal).normalized()
        _vel = ceiling_floor_tangent * _vel.dot(ceiling_floor_tangent)

    else:
        if vel.dot(ceiling_collision_normal) <= 0:
            var ceiling_floor_tangent = ceiling_collision_normal.cross(floor_collision_normal).normalized()
            _vel = ceiling_floor_tangent * _vel.dot(ceiling_floor_tangent)
    return _vel

walls:

func wall_collision_solver(_vel, delta):
    if (stair_step_success):
    else:
        on_wall = true
        if on_floor:
            if vel.dot(wall_collision_normal) <= 0:
                var wall_floor_tangent = wall_collision_normal.cross(floor_collision_normal).normalized()
                _vel = wall_floor_tangent * _vel.dot(wall_floor_tangent)
        else:
            _vel += current_gravity * delta
            _vel = _vel.slide(wall_collision_normal)
        if _vel.y > 0 and floor_collision_normal == Vector3.ZERO:
            _vel.y = 0 #prevent wall climbing
    return _vel

And that's it. The only thing left after this is to actually move along the new velocity that this code has deemed safe, and that is just move_and_collide(vel*delta) This code works great, but for some reason I don't understand, seems to be way more sensitive to getting 100% accurate normals than the default move_and_slide() is. All I know about move_and_slide() to this day is that is does something similar to me in the sense that it uses normals move_and_collide() and uses vector algebra to redirect the velocity.

I hope this helped clear up how a flipped normal can be incredibly noticeable, especially because this runs 240 times per second (to match my refresh rate since godot doesn't have physics interpolation done for 4.0 yet). A single wrong normal can spiral into completely changing the trajectory of the player. I hope I did a better job explaining myself than I did on here.

[Gamepro5]

I'm not sure if it's the exact same level as what's shown in @Gamepro5's YouTube video, or even the same controller, but something akin to that level is bundled with this project, so I replaced the dev_test node in game.tscn with maps/catwalk.tscn, moved the character's third-person camera a bit closer, enabled the physics debug drawing through "Visible Collision Shapes" in the editor's "Debug" menu

Yes, sorry about that. The game I'm making is called TeamSpec and it's a multiplayer FPS. The code that controls the character controller has been open sourced (so others don't need to spend 2 years learning vector math and writing it) and is the repo you found. The different maps are made for TeamSpec, but I often backport them to the character controller repo. TeamSpec is almost ready for release and the code is private, but even if it weren't, 90% of it isn't physics related. That video was recording on the TeamSpec godot project because I know that the only things that make it move this way can be found on https://github.com/Gamepro5/Custom_CharacterBody3D_Movement_System.

It was never my intention to have you guys see that project, because the issues I get always boil down to a faulty collision normal or unreported collision. I only use my character controller to test, because walking around a lot in it does show me where the jank is and I can typically boil it down to one move_and_collide() call at a specific position. I was under the assumption that my project was way below you, judging by how this is the Jolt Physics engine and my MRPS should thus exclude my character controller from the equation. It seems that my character controller is quite good at visually exposing bad normals, which was why I demonstrated it in the tiny YouTube video.

[jrouwe]

@Gamepro5 Thanks for the description! I would indeed prefer not to debug the actual character code, but I obviously want to fix faulty normals (as that could happen to anyone, anywhere). The reason I tried looking at your project is that I don't like saying for the 3rd time that something is fixed based on the MRPs given, only to find out a couple of hours later that it does indeed fix those cases but that there are many other broken cases.

What is unclear to me at the moment is if with this latest fix you're still seeing normals pointing downwards. Since there are at least 2 different issues, it would be nice to confirm that 1 of them is fixed (even though it might not actually result in a noticeable improvement of the character as a whole). If you still get a normal pointing downwards then I'd like to have another MRP for it (a smaller/larger part of a level + a single call to move_and_collide would be sufficient for that, I can boil that down to a single shape vs shape test).

If I may make a suggestion for an improvement: Shapes with sharp edges like cylinders and boxes are very error prone in collision detection (a cylinder is easily the most unstable shape of all shapes). Because the contact point is not a single point but a surface (e.g. rectangle or sphere) they are much more likely to give a jittering contact point or errors in contact normal. For the physics simulation, the contact point is often not used at all as the colliding faces are used to reconstruct the contact surface, so this jitter is not much of an issue. If you however rely on 100% accurate contact normals then a capsule or a perhaps a pencil shaped object is much more likely to work well:

[Gamepro5]

If you however rely on 100% accurate contact normals then a capsule or a perhaps a pencil shaped object is much more likely to work well:

That's interesting; I always thought a box would be the most efficient and bug free shape, because it has been used since quake.

I would indeed prefer not to debug the actual character code

Yes of course, I'm not asking for this nor did I expect it. It's why I was so insistent on not talking about my character controller as I knew it was unrelated to the core issue at hand.

What is unclear to me at the moment is if with this latest fix you're still seeing normals pointing downwards. Since there are at least 2 different issues

I will try your third fix and see if I can find a normal pointing downwards. I was pretty surprised that mihe beat me to it and managed to reverse engineer my youtube video, but I'll try to do the same testing I usually do (which basically boils down to printing position, velocity, and normals every frame to create an MRP that contains just one movement at a specific position).

[Gamepro5]

@mihe, about https://github.com/jrouwe/JoltPhysics/issues/671#issuecomment-1703845856, I downloaded @jrouwe's third patch and I haven't been able to see this behavior. Maybe my character controller is a tiny bit different to the backported one you used? I'm really confused. I'm going to keep testing but I haven't ran into any issues yet.

[mihe]

I'm only really able to provoke the issue with the ramps in these two rooms for some reason, and mostly at low speeds during deceleration:

But again, I'm not seeing any downwards-facing normals being reported, with or without #691, so even if what I ran into is a problem it's not likely to be resolved by #691 either way. Also, the severity of the problem I'm seeing seems to be inversely proportional to the amount of recovery iterations you set, so I would imagine this has more to do with the penetration depth being wonky when not using any shape margin.

But if you see an actual improvement when using #691, and you're not able to provoke any more issues, then that's some sort of win I guess. No need to let irrelevant edge-cases haunt us.

[jrouwe]

Yes it does sound like a win! (and a desperately needed one 😃).

@Gamepro5 Let me know if you're done testing this and then I'll merge the change into master.

[Gamepro5]

Yeah, I've been testing on that map for a while and don't seem to be able to reproduce any issues like that. I think the version of my character he was testing it on is a bit outdated, but considering it doesn't seem to do anything wrong on my current one, I think it's totally fine. Github isn't letting me download my older one, so I can't really confirm why mihe and I had different results.

I think it's safe to merge!