pokepetter / ursina

A game engine powered by python and panda3d.
https://pokepetter.github.io/ursina/
MIT License
2.23k stars 328 forks source link

Using more than one boxcast in a single update breaks some of the boxcasts. #665

Open LOUKOUKOU opened 8 months ago

LOUKOUKOU commented 8 months ago

I am busy rewriting ursina.prefabs.platformer_controller_2d

In the update function, you can see it uses boxcast 3 times (the last time doesn't matter as much) .

https://github.com/pokepetter/ursina/blob/c587be243ce04aeed918af4d21a283842c76076f/ursina/prefabs/platformer_controller_2d.py#L80

I've refactored it to this, we have a boxcast for the sides, and we have a boxcast for the bottom.

    def update(self):
        verticalRay = boxcast(
          self.position+Vec3(self.velocity * time.dt * self.walk_speed,self.scale_y/2,0),
          # self.position+Vec3(sefl,self.scale_y/2,0),
          direction=Vec3(self.velocity,0,0),
          distance=abs(self.scale_x/2),
          ignore=(self,),
          traverse_target=self.traverse_target,
          thickness=(self.scale_x*.99, self.scale_y*.9),
          debug=True
        )
        bottomRay = boxcast(
          self.world_position+Vec3(0,.1,0),
          self.down,
          distance=max(.15, self.air_time * self.gravity),
          ignore=(self, ),
          traverse_target=self.traverse_target,
          thickness=self.scale_x*.9,
          debug=True
        )

        if not verticalRay.hit:
          self.x += self.velocity * time.dt * self.walk_speed
        if bottomRay.hit:
            if not self.grounded:
                self.land()
            self.grounded = True
            self.y = bottomRay.world_point[1]
            return
        else:
            self.grounded = False

        if not self.grounded and not self.jumping:
            self.y -= min(self.air_time * self.gravity, bottomRay.distance-.1)
            self.air_time += time.dt*4 * self.gravity

As you can see, the debug is true. However, it will only show one at a time, depending on which one I instantiate last (it takes preference to the last one).

There's also weird behavior with the bottom cast when the vertical cast is in, even though they are unrelated. I'm linking a video to explain.

Screen Recording - Made with FlexClip (online-video-cutter.com).webm

In this video, you can see that it does this weird thing where it bounces up and down, when I comment the boxcast that has to do with the sides (not the bottom), it then stops doing it. Also, as you can see, it only shows one debug indicator at a time.

I'm using vscode on windows.

LOUKOUKOU commented 8 months ago

I think I've narrowed down the issue to 2 things.

Firstly, if you look at the boxcast function, you'll see that it reuses the same _boxcast_box variable to measure all its collisions.

This is done for optimization reasons, there is, however, a problem here. There are some properties, I think something to do with the look_at function, that persist between boxcast calls. This makes it so that subsequent calls are affected by previous ones.

In the boxcasts function, I changed line 24 from _boxcast_box.look_at(origin + direction) to _boxcast_box.lookAt(origin + direction), and it seemed to fix it. The lookAt function, is a panda3d function that the Entity class inherited. This isn't a final fix, just a possible indicator that the problem might lie in the look_at function.

There is also a broader issue, that has to do with coding standards. The issue is that the boxcast function isn't pure, which is a problem if it's a static function. I would suggest 2 approaches to fixing this from a best practices point of view. The first would to be just simply make boxcast a class, and then create an instance for ray you want to cast, and have the memory be properly managed within the class. The second would be to have the boxcast function recreate the _boxcast_box variable inside its function scope each time, and give the programmer the option to pass in their own _boxcast_box variable, that is reused, if they wished to save memory.

LOUKOUKOU commented 8 months ago

I think there might be some unforeseen consequences when calling the boxcast function Here you call the look_at, but you don't pass an up parameter in. https://github.com/pokepetter/ursina/blob/c587be243ce04aeed918af4d21a283842c76076f/ursina/boxcast.py#L23-L25 In the function, it then uses the entities up, instead of a passed in up https://github.com/pokepetter/ursina/blob/c587be243ce04aeed918af4d21a283842c76076f/ursina/entity.py#L943-L946