Skip to content

Conversation

@jhame101
Copy link

@jhame101 jhame101 commented Aug 11, 2020

I've created a new folder in the UnrealGEARS folder called LammpsEditor425 (and renamed the existing folder to LammpsEditor416). There are a few changes. First, I took out the controller static mesh and replaced it with the built in controller visualization (which isn't limited to the Vive controller and will automatically detect which controller is being used) and second, I changed the controller bindings. In UE4.25, you can't use generic bindings like "motion controller trigger"; instead you must specify what type of controller you're referring to. I tested with a Vive controller and it should also work with an Occulus touch controller. All these changes were only in version 4.25, and I left version 4.16 mostly unchanged.

@KeijiBranshi
Copy link
Collaborator

These look like a lot of great updates! There's quite a lot of files changed in this PR though (especially configs). Is there anyway it could be split up to make the diff a little easier to consume? I'm not exactly sure what the standard practice is for big UE updates like this.

cc @KenichiNomura @aiichironakano

@jhame101
Copy link
Author

I think that any differences made in the 4.16 folder could be undone without consequences (my intention was to leave it alone, but with all my looking back at it I changed some things). Other than that, I don't think it could be broken up a whole lot since the updated version didn't work properly until a86e6fd. I'll recopy all the files in LammpsEditor416 to be how they were before I changed them and see if that makes it any better.

@jhame101
Copy link
Author

It's cleaner now. I can't do much more (that I know of) to make it any better, though, since copying everything is going to make a large diff no matter what.

@KeijiBranshi
Copy link
Collaborator

@jhame101 thanks for cleaning it up a bit. Would you also be able to post a list of the full filepaths you changed? That would just help as we read through the diff. I know you mentioned it in your first comment, but I still think it'd be helpful to have them explicitly laid out.

@jhame101
Copy link
Author

jhame101 commented Sep 9, 2020

@KeijiBranshi Sure. Is this what you mean?
UnrealGEARS/LammpsEditor/* → UnrealGEARS/LammpsEditor416/*
+ UnrealGEARS/LammpsEditor425/*

@KeijiBranshi
Copy link
Collaborator

@KeijiBranshi Sure. Is this what you mean?
UnrealGEARS/LammpsEditor/* → UnrealGEARS/LammpsEditor416/*

  • UnrealGEARS/LammpsEditor425/*

I was thinking a bit more fine-grained. For instance:

  1. UnrealGEARS/LammpsEditor425/Config*:
    • No Changes. Directly Copied from 4.16
  2. UnrealGEARS/LammpsEditor425/Source/LammpsVR/Classes/LammpsController.h:
    • Changed m_lammpsOpenNoMPI to do XYZ

Just to help add a bit more context to all the points here: #4 (comment)

@KeijiBranshi
Copy link
Collaborator

Synced offline with @KenichiNomura and @aiichironakano .

@jhame101 , we're wondering if you want to just check this into the original folder (rather than split it up by 416 and 425)? That would keep the repo size to a minimum (considering its already fairly large) and set a more scalable precedent as Unreal Engine continues to get updated.

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.

2 participants