Skip to content

Move MovieWriterMJPEG class to jpg module it depends on #106013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented May 2, 2025


I don't intend to work on it further, but there are various UX issues I noticed which may be worth addressing (@Calinou):

  • With this PR when using .avi extension with the jpg module disabled, and likely already before when using any other extension than .avi or .png, the movie writer doesn't give any warning and starts, but prints an error:
ERROR: Can't find movie writer for file type, aborting: res://test.avi
   at: setup2 (main/main.cpp:3575)

It should probably validate that the file type is supported before running the project in movie writer mode, showing explanations to the user with a popup.

  • When no movie writer write path is configured, the following dialog is shown. It has multiple UX issues.
    image

    • The text alignment is weird with the manual wrap on the second-to-last line.
    • It suggests two options to set a movie path but not the main one, which is the editor/movie_writer/movie_file project setting.
    • For a feature that has such a prominent button in the run bar, the first time experience is quite bad when you get this dialog. Similar to how we improved selecting a main scene when running for the first time, this dialog should likely give an easy option to set a default movie path project setting (e.g. derived from the project name, writing to res://).
  • The expectation to select the output format (avi or png) just based on the file extension, with the only information available in the documentation for the project setting, is suboptimal. In the future, such features might be better served with a dedicated dialog (similar to the "Customize Run Instances..." dialog) that lets users configure all the settings for encoding.

@akien-mga akien-mga added this to the 4.5 milestone May 2, 2025
@akien-mga akien-mga requested a review from Calinou May 2, 2025 09:03
@akien-mga akien-mga force-pushed the move-MovieWriterMJPEG-to-jpg-module branch from 6b35f26 to de31031 Compare May 2, 2025 10:05
@akien-mga akien-mga marked this pull request as ready for review May 2, 2025 10:05
@akien-mga akien-mga requested review from a team as code owners May 2, 2025 10:05
@Calinou
Copy link
Member

Calinou commented May 2, 2025

  • With this PR when using .avi extension with the jpg module disabled, and likely already before when using any other extension than .avi or .png, the movie writer doesn't give any warning and starts, but prints an error:

I have a PR that aims to address this, but I ran into an issue: #95867

  • When no movie writer write path is configured, the following dialog is shown. It has multiple UX issues.

We can probably reuse the "no default scene has been configured" dialog's code for this.

Edit: I have a WIP branch that aims to do this, but I get a crash when the dialog appears (I don't know why, the backtrace doesn't tell me much): https://github.com/Calinou/godot/tree/editor-moviewriter-ask-for-movie-file

@akien-mga akien-mga force-pushed the move-MovieWriterMJPEG-to-jpg-module branch from de31031 to 0bec705 Compare May 9, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MovieWriterMJPEG doesn't work with module_jpg_enabled=no
3 participants