Closed GandaG closed 8 years ago
Hey, Just wondering if this work is planned on being merged in to the main branch?
If you review it and post any bugs you find here I'm sure it'll go much faster :+1:
Hi!
I should have written about this before...
I haven't really used UnitySteer on 2D, so I feel a bit hesitant about merging it in since I likely won't be supporting it myself because of the lack of my own testbed.
@GandaG, if you can commit to support it, I'll be happy to give you contributor access.
I will be using it for 2D also, I am happy to review or help in any way I can guys (not for a few days being xmas).
I have 1 comment on it in general @GandaG. Sorry, but I'm not so sure about the code duplication. It is pretty much the same logic but supporting 2D components instead of 3D. I can totally see why it was done that way though and I can't right now think of a nice way to support both without over complicating the library.
@ricardojmendez Does pull request #28 still make sense if this change is merged?
@pjohalloran The split idea was from a reddit post (just search for unity steer) that rightly complained about code readability. That way I can add little features to the 2D part without messing with the 3D behaviours (like tagging, labeling, etc.). Also, users don't need to go changing directives in source code.
@ricardojmendez Sorry, can't really commit to anything right now, but I can and will help out as much as I can.
Ah, yeah, I found the post you mentioned, Kk. I'm going to give it a try @GandaG
I am having a look today @GandaG. Looks good to me in a very small test project i am working on.
I would suggest some more work before it gets merged in though. I am happy to help with this changes if you agree.
1 2D components in Unity follow the convention ComponentName2D. For example, see RigidBody and RigidBody2D. I suggest we rename all the 2D UnitySteer components to follow this convention. 2 We update the UnitySteerExamples project also to demonstrate the UnitySteer 2D components as well as the 3D components. 3 We add a Vector2ToggleAttribute to make the inspector for the 2D components not mention the z component of the transform and update the 2D code to use it. 4 We remove the Behaviour2DAttribute as it would not be needed I do not think, if the naming conventions step 1 above be followed.
What do you think @GandaG and @ricardojmendez ?
@pjohalloran
Would you mind doing it? I really don't want to wait 2 hours to install unity :/ if @ricardojmendez agrees obviously.
I am in the middle of doing 1 3 and 4 right now. I will commit to my fork of UnitySteer and can do a pull request into your fork as soon as i am done. Then you could maybe update the PR here?
Sorry, I am confused by your reply to 2. You don't think we should update the UnitySteer examples?
@GandaG Have made a pull request with changes above to your fork. See here https://github.com/GandaG/UnitySteer/pull/1
Changes suggested above are in. I guess the only question to resolve now is if this can be merged to main branch @GandaG and @ricardojmendez ? I am happy to help @GandaG maintain the 2D code.
Hi @pjohalloran & @GandaG,
I'm mostly offline today - just scanned through the exchange but haven't had time to go over the commits. I did want to chime in and let you know I'm around and checking things.
I'll need to reinstall Unity 5 on this machine, so review will need to wait until Monday, but I'll get back to you then.
Thanks for volunteering!
@GandaG @pjohalloran: Can you confirm if you've created this on Unity 4.x or Unity 5? I'm not that familiar with Unity's 2D APIs, so can't readily tell.
I made my changes on Unity 5.3.1.f1.
Cool, I like the approach of using a separate set of behaviors. Metas seem to be intact against the old ones, as I can import this into UnitySteerExamples without any problems.
Tested against Unity 4.x and it looks like this would be Unity 5-specific, as 4.x's Vector2 does not have a Reflect
method.
(I really wish Unity would tag APIs with the version at which they became available.)
I'll likely end up tagging this as 3.1, since it requires a different Unity version.
Two questions:
Maybe it might be a good idea to have a seperate 2D folder of examples in UnitySteerExamples, that match the 3D versions?
Yes, that sounds like a good approach.
Ok, I do not yet have any examples to submit but i'd be happy to update the UnitySteerExamples repo with a pull request for that. Probably wont get around to it until next weekend.
@ricardojmendez I wonder are you available for questions/advice on how to use UnitySteer effectively?
@pjohalloran In general I am, but it depends greatly on how busy I am at the time, and how implementation-specific the question is.
As you've probably found, solutions are rarely black and white, and it can require some time of playing with the values to get the exact behavior you want, which is often hard to describe if one's not working on the project. In such a case, chances are the answer will take the shape of something like this post.
Thats good. Yes thats true - its not often easy unless you have knowledge of the project. I don't have anything specific to ask right now but possibly might in the near future. Thank you.
Added a pull request for the UnitySteerExamples for the 2D behaviours. See https://github.com/ricardojmendez/UnitySteerExamples/pull/4
Moved 2D files to a new folder, created 2D namespace and added a little label to 2D components to make sure people don't mix them.
Pretty much all that changed were specific 2D physics components (like Rigidbody to Rigidbody2D and removed references to CharacterController - the old characterevasion script is now a rigidbodyevasion) and methods (OverlapSphere to OverlapCircle).
Also changed Vector3 refs to Vector2 - it should work with Vector3, but better safe than sorry.
Added a custom Forward vector to vehicle components that allows you to choose which direction your sprite is facing.
Slightly improved menu hierarchy for 2D files.