arcade icon indicating copy to clipboard operation
arcade copied to clipboard

Camera new implementations

Open alejcas opened this issue 2 years ago • 3 comments

This pull request adds two new cameras to arcade.

First a BaseCamera that is just a camera that allows to set the viewport, projection and can move around. Just that Second a AdvanceCamera that implements a camera with more functionality.

The arcade current Camera remains the same. In the future a deprecation warning should be implemented to drop this camera in favor of the AdvanceCamera (or whatever other solution you may find appropriate).

The changes (differences also) are:

  • Both cameras implement viewport and projection independent from each other
  • Both cameras allow to set the camera viewport on whatever position other than left, bottom = (0, 0)
  • Both cameras allow to set the camera projection on whatever position other than left, bottom = (0, 0)
  • Both cameras only calculate the matrices when it's needed. arcade.Camera calculates the matrices in every frame.
  • Both cameras allow to set important properties right from the init.
  • AdvanceCamera has a zoom (renamed from the more abstract keyword "scale")
  • AdvanceCamera has a center method that just centers (moves to the center) the camera on a certain vector
  • AdvanceCamera has a get_map_coordinates method that returns the "map" (or whatever you call what the camera is navigating on) coordinates from the camera viewport coordinates relative to the screen.

AdvanceCamera also has rotation but that was already implemented on the development branch.

What's missing:

  • Although I test them, further testing on the cameras is needed. Specially when using more than one camera and when cameras are positioned far away of left, bottom (0, 0)
  • Collision detection when using rotation or zoom doesn't work. @einarf told me on discord that this should be possible to overcome by implementing the arcade collision methods on the camera and translating the coordinates to check with the projection and view matrices. I have not achieved a success here. Also seems trivial to achieve get_sprites_at_point but I can image how to transform coordinates when checking a spritelist against another spritelist for example.

I hope this PR helps and gets accepted.

alejcas avatar Sep 08 '22 21:09 alejcas

The PR also changes:

  • application.py to change the type definition of window.current_camera
  • init to add the imports for BaseCamera and AdvanceCamera

alejcas avatar Sep 08 '22 22:09 alejcas

#1076

einarf avatar Sep 09 '22 00:09 einarf

Still can't figure out what's failing in collision detection.

This is a preliminary method based on @einarf comments on discord:


# method from arcade.AdvanceCamera class

def get_sprites_at_point(self, point: tuple, sprite_list: "arcade.SpriteList"):
    # this is needed when the camera applies rotation or zoom.
    # default arcade.get_sprites_at_point will fail to get the sprites

    x, y = point
    vector = Vec4(x, y, 0, 1)

    # self._view_matrix takes the zoom into account (not rotation)
    # in theory this should translate the point to the correct position when zoom is applied
    
    translated_point = self._view_matrix @ vector
    return arcade.get_sprites_at_point(translated_point, sprite_list)

It is not working and this is what I think about (although I can be very wrong):

  • camera._view_matrix takes zoom into account but not rotation. Rotation is set inside camera._rotation_matrix (this is what is passed then to window.view_matrix_2d). But rotation matrix does not take zoom into account... so to make collision rotation work both rotation and zoom matrices should be taken into account.
  • the sprite_list should also somehow translate it's coordinates?

alejcas avatar Sep 15 '22 14:09 alejcas

Creating a new camera and deprecating an old one, I think needs a strong reason for the switch. All examples and docs need to be updated, and users will need to update their code. Are the features of the new camera compelling enough for this?

I'm not sure yet that I see those reasons.

pvcraven avatar Sep 23 '22 20:09 pvcraven

The fact is that the current arcade camera has some things that must be changed. Mainly the fact that viewport and projection are interrelated in some ways that are incorrect. Or the fact that camera viewport and projection is hardcoded at (0,0) left bottom.

Why I didn’t just change/fix/add features to the old camera? Because to do that I needed to change the constructor method signature + change some things that will break old code. And that is a mess for current users.

As a user of arcade I find the current implementation of the Camera lacking some big big things as I mention on the changelog of this PR. And that was the rationale…

Deprecating the old camera was just a suggestion, you don’t have to do it as I tried to say with “or whatever other solution you may find appropriate”.

alejcas avatar Sep 23 '22 20:09 alejcas

Sorry for not reviewing this yet. Something definitely needs to be fixed with the camera. We are completely stuck with the current one unless we do breaking changes.

einarf avatar Sep 25 '22 18:09 einarf

Hi @einarf Can I do something with this to get the functionality merged (in it's current state or other approach)?

alejcas avatar Oct 10 '22 13:10 alejcas

Let's just merge this in for now so we get some progress. There are probably some things to iron out before 2.7 release. Things I don't have the brain power and time to evaluate right now 😄 (Texture revamp stuff takes up 100% of my energy atm)

einarf avatar Oct 10 '22 17:10 einarf