Skip to content

Conversation

@AlrIsmail
Copy link
Contributor

@AlrIsmail AlrIsmail commented Feb 27, 2025

This PR addresses issue

I’ve replaced the GIFs with WebM videos to eliminate visual artifacts.

@AlrIsmail AlrIsmail changed the title Fix virtual artifacts Fix visual artifacts Feb 27, 2025
@skyace65 skyace65 added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Feb 28, 2025
Copy link
Contributor

@skyace65 skyace65 left a comment

Choose a reason for hiding this comment

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

I built this locally and everything looks good

@skyace65
Copy link
Contributor

skyace65 commented Mar 2, 2025

@tetrapod00 do we have any preference one way or the other for using the :nocontrols: flag with videos? It pretty much does what it says, hides the bottom bar and stop/start button.

@tetrapod00
Copy link
Contributor

tetrapod00 commented Mar 2, 2025

There's no precedent for using :nocontrols: yet, so we're making the decision whether or not to use it right now.
Pros:

  • Matches GIF functionality exactly

Cons:

  • Less accessible and useful by removing the ability to pause or scrub
  • Other videos don't currently use it, no precedent for it
  • One more thing to include when adding new videos, one more thing that may break with future updates to the video library

I would make the call to remove :nocontrols: and avoid introducing it in the future

Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

Built it locally both with and without :nocontrols:. I would prefer these videos to have controls, even leaving aside the other reasons to avoid introducing :nocontrols:.

@skyace65
Copy link
Contributor

skyace65 commented Mar 2, 2025

Applied tetrapods changes to remove :nocontrols:

@skyace65 skyace65 merged commit 743d4d7 into godotengine:master Mar 3, 2025
2 checks passed
@skyace65
Copy link
Contributor

skyace65 commented Mar 3, 2025

Thanks! And congrats on your first merged PR! In the future though we'd recommend forking your master branch instead of making changes directly to it, that makes keeping it updated difficult. If you want to know more we have a git guide here: https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants