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.

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

Refactored remaining Vec2 uses to use Vector2 #48

Open
wants to merge 1 commit into
base: v2.4
Choose a base branch
from

Conversation

thomasvt
Copy link
Collaborator

@thomasvt thomasvt commented Jan 9, 2023

Fixes # 46
#46

I replaced all remaining uses of Vec2 with Vector2. Vec2 is deleted. This is a breaking change, as keeping the old overloads (in class DebugDraw) around didn't make much sense to me. Please advise.

I had issues with project references in both the main and ExamplesWindow solutions: project references to projects that aren't in the sln. I took the liberty to add those to the sln to allow me to build. But putting everything in one solution instead of two (current situation) is probably more elegant, if they need eachother anyway.

@thomasvt
Copy link
Collaborator Author

thomasvt commented Jan 9, 2023

I tried adding unit-tests for DebugDraw but it got ugly fast because it's not easy to mock it. So there are no unittests, but the ExamplesWindow works.

@@ -47,33 +46,26 @@ public void ClearFlags(DrawFlags flags)
/// <summary>
/// Draw a closed polygon provided in CCW order.
/// </summary>
#pragma warning disable 618
[Obsolete("Look out for new calls using Vector2")]
Copy link
Collaborator

@HughPH HughPH Jan 10, 2023

Choose a reason for hiding this comment

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

Instead of deleting, can you change the description to read "Use Vector2 overload instead" and set the Error flag to true? Then anyone already using these with Vec2 will get guidance as to how to change their code, and we can delete them at a later date.

You would need to create stub methods with no implementation for the old signatures.

@@ -1,40 +1,49 @@

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, why box2d-netstandard.sln was changed? It works by keeping it as is. I'd vote to leave it unchanged.

Projects in ./examples and ./src are separated projects, and only ./examples projects can reference ./src projects. If you'd like to have one project solution file, then please just create box2d.sln in the root directory.

@@ -245,7 +245,7 @@ public void DrawCircle(Vec2 center, float radius, Color color)
});
}

public void DrawSolidCircle(Vec2 center, float radius, Vec2 axis, Color color)
public void DrawSolidCircle(System.Numerics.Vector2 center, float radius, System.Numerics.Vector2 axis, Color color)
Copy link
Owner

Choose a reason for hiding this comment

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

I'd just just Vector2 instead of longer System.Numerics.Vector2 because it helps to keep it shorter line here. It's up to you to decide! :)

@@ -1,72 +1,92 @@

Copy link
Owner

@codingben codingben Jan 14, 2023

Choose a reason for hiding this comment

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

Same here, sorry. Why box2d-examples-window.sln was changed? It works by keeping it as is. Do you use dotnet build from CLI?

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.

3 participants