Skip to content

Conversation

pushfoo
Copy link
Member

@pushfoo pushfoo commented Oct 24, 2024

  1. Stop trying to set the window's read-only clock value
  2. Convert them to the new View example style
  3. Add entries for them in the examples listing

* Stop trying to set the window's read-only clock value

* Fix annoying indent on list at top of comments
@pushfoo pushfoo requested a review from DragonMoffon October 24, 2024 08:48
Copy link
Collaborator

@DragonMoffon DragonMoffon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The window has a depth buffer by default, and spritelists innately support depth testing so this should be moved to examples

* Make the example subclass view

* Use the same style found in other examples

* Grab the context locally
@alejcas
Copy link
Member

alejcas commented Oct 24, 2024

Seems like this line is wrong:

if self.use_depth:

You moved use_depth to the window so the line should check:

if self.window.use_depth:

Anyway I don't understand why you moved that attribute to the window. Just leave it in the view...no?

@alejcas
Copy link
Member

alejcas commented Oct 24, 2024

Seems like this line is wrong:

if self.use_depth:

You moved use_depth to the window so the line should check:

if self.window.use_depth:

Anyway I don't understand why you moved that attribute to the window. Just leave it in the view...no?

I think this is what's wrong:

window.use_depth = not window.use_depth

* Convert to View style

* Make bg / clear color a Color defined at the top of the file

* Use WINDOW_TITLE, WINDOW_WIDTH, and WINDOW_HEIGHT constants
* Add a screenshot for the example page and example gallery

* Add an .rst file to reference the code and example

* Add the .rst file to the examples gallery
* Add a screenshot for the page and gallery

* Add an example .rst file for it

* Add an entry in the examples page
@pushfoo
Copy link
Member Author

pushfoo commented Oct 24, 2024

@alejcas fixed

@pushfoo pushfoo changed the title Fix sprite depth example clock and style Promote sprite depth examples from experimental to main Oct 24, 2024
@pushfoo pushfoo requested a review from DragonMoffon October 24, 2024 10:56
@pushfoo pushfoo requested a review from DragonMoffon October 25, 2024 16:44
@einarf
Copy link
Member

einarf commented Oct 25, 2024

The reason it wasn't moved into examples was a lingering issue related to the depth range. As far as I know the depth range set in the ortho projection is [-1.0, 1.0] defining the depth that can be set in the sprite's geometry.

We could possibly change to to [-1, 100] to give the user a more reasonable range to work with? If the sprite depth is set outside this range it risks being clipped in primitive assembly not appearing.

Including a new reasonable depth range would make things a bit more cleaner?

@einarf
Copy link
Member

einarf commented Oct 25, 2024

It seems @DragonMoffon changed the range to [-100, 100] so we should make sure to mention this range in the example and take advantage of this range if that's not already in place. Seems it was modified a bit from the original -1 to 1 range.

The default depth range in arcade is -100 (near plane) / 100 (far plane). Higher value is further away from the camera.

@pushfoo
Copy link
Member Author

pushfoo commented Oct 25, 2024

Can we add default near and far constants to make this more legible?

@einarf
Copy link
Member

einarf commented Oct 25, 2024

For now I think it's enough to just mention the default range.

@DragonMoffon
Copy link
Collaborator

I didn't actually choose this range myself. There was definitely a code snippet in Arcade that I based it on. I feel like it was the internal matrix gen, but it might have been one of the gl tutorials.

In any case, in the depth example, it should probably be mentioned since it's directly settable in Camer2D's init

Copy link
Collaborator

@DragonMoffon DragonMoffon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last, very pedantic, change. Im otherwise happy for it to be merged.

@pushfoo
Copy link
Member Author

pushfoo commented Oct 26, 2024

  • Added doc about near / far clipping
  • Changed time phrasing to be lessspecific to American / Hemmingway style
  • Centralized near / far defaults from a single source of truth

@pushfoo
Copy link
Member Author

pushfoo commented Oct 26, 2024

"yeah looks good" - DragonMoffon (Discord)

@pushfoo pushfoo merged commit e554b02 into development Oct 26, 2024
9 checks passed
@pushfoo pushfoo deleted the sprite-depth-clock-fix branch October 26, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants