Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve motor demo #144

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve motor demo #144

wants to merge 2 commits into from

Conversation

i-make-robots
Copy link

  • comments for clarity
  • commands now stack to increase and decrease velocity.
  • increased maximum force-per-frame to each joint
  • simplified nearcallback with lambda and removed some other redundancies

i-make-robots
i-make-robots commented Nov 4, 2024
View reviewed changes
demo/src/main/java/org/ode4j/demo/DemoMotor.java

private static void nearCallback (Object data, DGeom o1, DGeom o2)
{
private void nearCallback (Object data, DGeom o1, DGeom o2) {
// exit without doing anything if the two bodies are connected by a joint
Copy link
Author

Choose a reason for hiding this comment

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

This comment is misleading. Was it supposed to be a TODO?

Copy link
Owner

Choose a reason for hiding this comment

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

The comment is not from me. I will have a look.

@tzaeschke
Copy link
Owner

Hi @i-make-robots, thanks a lot for the PR!

As I understand, this is a purely cosmetic PR, correct?

Unfortunately, there is a problem, I usually do not accept cosmetic changes to ode4j. I know that the look of the code makes your toenails curl up, but there is a reason I keep it this way. ode4j is an ongoing port of ODE C++. Every bit of rearranging/adding/removing of the Java code makes it harder for all following iterations when I port the latest changes.
It is not so bad if ODE has few changes, but they sometimes refactor their code, and then it becomes very hard because I have to compare Java and C++ side by side and for every difference I have to track back where it came from. Basically, I have been there many times and decided to only fix the most obvious (e.g. typos) things.

The fact that this is a demo (which is looked at by users) means that I may actually adopt some changes, but it is likely only a small subset of what you propose. I will have a look. Sorry about that.

I really do appreciate the effort though.

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

@tzaeschke tzaeschke tzaeschke left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@i-make-robots @tzaeschke