Skip to content

Issue - 3006#3040

Open
Ryon-2026 wants to merge 8 commits intof3d-app:masterfrom
Ryon-2026:Issue-3006
Open

Issue - 3006#3040
Ryon-2026 wants to merge 8 commits intof3d-app:masterfrom
Ryon-2026:Issue-3006

Conversation

@Ryon-2026
Copy link
Copy Markdown

Describe your changes

Issue ticket number and link if any

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

AI Disclosure

  • I did not use AI to generate any of the content of that pull request
  • I used AI to generate code in that pull request, if yes please disclose which part of the code was generated and with which model.
  • ...

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

@Ryon-2026 Ryon-2026 changed the title Second Try Issue - 3006 Apr 15, 2026
@mwestphal mwestphal self-requested a review April 15, 2026 07:00
Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

thanks for opening this PR, please request a review whenever you need :)

@Ryon-2026
Copy link
Copy Markdown
Author

\ci fast

@mwestphal mwestphal self-requested a review April 15, 2026 07:09
Comment thread doc/user/07-COMMANDS.md Outdated
`roll_camera value`: A specific command to roll the camera on its side, takes an angle in degrees as an argument.
eg: `roll_camera 120`.

`set camera_position x y z`: A specific command to set the position of the camera to the position of x y z.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there should be a single set_camera command that suppot both x,y,z and top/bottom

Copy link
Copy Markdown
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

wrong impl

@Ryon-2026 Ryon-2026 requested a review from mwestphal April 29, 2026 06:17
@Ryon-2026
Copy link
Copy Markdown
Author

@Meakk could you review this? Its a bit late and I cant figure out how to request anyone outside of the suggested users to review so thats why im writing this over comment.

Thank you!

Copy link
Copy Markdown
Member

@Meakk Meakk left a comment

Choose a reason for hiding this comment

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

Looks ok. However I'm wondering if it shouldn't be a new command set_camera_position instead of extending the existing set_camera.
You will also need to create an application test similar to TestCommandScriptSetCameraLeft

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please remove this file

{
this->Internals->SetViewOrbit(internals::ViewType::VT_ISOMETRIC);
this->Internals->Style->ResetTemporaryUp();
check_args(args, 3, "set_camera");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

check_args calls here and above are not needed anymore.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants