AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.41k stars 288 forks source link

Consider Incorporating FarseerDuality Project Into Main Package / Solution #570

Closed ilexp closed 6 years ago

ilexp commented 7 years ago

Summary

Duality uses a custom Farseer Physics port, which is built and deployed separately. This not only increases iteration times heavily, but it also makes Duality-specific specialization harder. Consider moving the FarseerDuality project into main and renaming it to Duality.Physics, paving the way for gradually improving and streamlining it for integrated Duality usage.

Analysis

Jared-Miller commented 6 years ago

Started merging the Farseer code into the main Duality project. For now, I left all the package names as is and put the Farseer code into Source/Core/Physics named DualityPhysics, to match the way primitives was structured.

@ilexp

ilexp commented 6 years ago

How would you prefer to handle the assembly information for the merged project?

I'm not sure I understand the question, but the goal would be that the DualityPhysics project is later released as its own core dependency NuGet package similar to DualityPrimitives.

Do you want the global .editorconfig styling changes applied to the DualityPhysics project?

Yes, but not necessarily in the first iteration. I would be fine if this was done separately after this issue is resolved, or right before closing it.


As far as I could see in a quick branch comparison, it seems like you based your branch off of master - however, as we're dealing with breaking changes here, we can't release this update as a regular v2.x update. Instead, please branch off of develop-3.0 in order to deal with this issue.

Also (assuming that you're new to the project) be aware that this issue is not tagged as Help Wanted mainly because I expect complications and structural decisions that are easier done when bringing along deeper knowledge of the project and its internals. It might not be an ideal choice for a first contribution. If you're looking for a good one to start, besides the Help Wanted tag also look out for those tagged with Good First Issue - we don't always have one of those waiting, but they're ideal starters.

In any case, let me know if you want to continue work on this. I definitely do not want to discourage your efforts and I really appreciate you want to contribute, but I also want to make sure new contributors get a proper learning curve and don't too many walls in their first task.

Jared-Miller commented 6 years ago

I'm not sure I understand the question, but the goal would be that the DualityPhysics project is later released as its own core dependency NuGet package similar to DualityPrimitives.

Sorry, I was wondering if you would want the assembly information to be changed (title from Farseer Physics Duality to Duality Physics, version from 4.1.4 to 3.x.x, etc). Based on your response, that appears to be what you were thinking. I've modified the AssemblyInfo and nuspec information to match what was done in primitives.

As far as I could see in a quick branch comparison, it seems like you based your branch off of master - however, as we're dealing with breaking changes here, we can't release this update as a regular v2.x update. Instead, please branch off of develop-3.0 in order to deal with this issue.

Yes, that should've been from develop-3.0. Changed the base.

In any case, let me know if you want to continue work on this. I definitely do not want to discourage your efforts and I really appreciate you want to contribute, but I also want to make sure new contributors get a proper learning curve and don't too many walls in their first task.

I don't mind some walls. I saw this in the list of 3.0 milestone items and it seemed like the upfront work was relatively simple (moving and renaming files, projects, and packages). Once that's completed, any other issues being blocked by the separation of the projects could be completed in 3.0.

ilexp commented 6 years ago

Sorry, I was wondering if you would want the assembly information to be changed (title from Farseer Physics Duality to Duality Physics, version from 4.1.4 to 3.x.x, etc). Based on your response, that appears to be what you were thinking. I've modified the AssemblyInfo and nuspec information to match what was done in primitives.

Ah. Yes, that was the plan. Now that you mention it though, we will need to make sure it is absolutely clear that this package / library was originally based on FarseerPhysics.

Yes, that should've been from develop-3.0. Changed the base.

👍

I don't mind some walls. I saw this in the list of 3.0 milestone items and it seemed like the upfront work was relatively simple (moving and renaming files, projects, and packages). Once that's completed, any other issues being blocked by the separation of the projects could be completed in 3.0.

Alright then 🙂 Let me know how things are going and whether you need more information on anything. Also, feel free to open up a PR for this early if you need a review.

Jared-Miller commented 6 years ago

Now that you mention it though, we will need to make sure it is absolutely clear that this package / library was originally based on FarseerPhysics.

That was the concern. I don't know if any of the information in the assembly itself is appropriate for that, though. Each of the source files still contains the original notice and the license information from FarseerDuality will be in the Physics directory. The nuspec definition currently indicates that it is a custom version of Farseer. Perhaps that is the best place to indicate that it is a fork, using the title, description, and/or summary?

ilexp commented 6 years ago

That was the concern. I don't know if any of the information in the assembly itself is appropriate for that, though. Each of the source files still contains the original notice and the license information from FarseerDuality will be in the Physics directory.

We should definitely keep the copyright and license notice in the subfolder that contains the project. Maybe add a local readme.md too, where we explain that this is still a custom Farseer variant.

The nuspec definition currently indicates that it is a custom version of Farseer. Perhaps that is the best place to indicate that it is a fork, using the title, description, and/or summary?

Mentioning it in both the description and summary sounds good. Keep it short for the summary, maybe add as a separate paragraph in the description.

Jared-Miller commented 6 years ago

Maybe add a local readme.md too, where we explain that this is still a custom Farseer variant.

There was already a readme there, so I'll update it to include the new name (Duality.Physics) and keep the reference to the original repository.

I'll start renaming the folders in Source/Core and Source/Editor tomorrow, if you haven't changed your mind on that. Was there anything I hadn't listed explicitly in the PR that you believe should be part of this issue resolution?

ilexp commented 6 years ago

I'll start renaming the folders in Source/Core and Source/Editor tomorrow, if you haven't changed your mind on that.

Please skip that for now - in fact, I hadn't made up my mind about them in the first place, and I'm not quite sure it would be a good idea. It was more a general thought that could be explored, rather than a work item or task.

Was there anything I hadn't listed explicitly in the PR that you believe should be part of this issue resolution?

Will get back to you on that as soon as I find time to review your PR progress :)

ilexp commented 6 years ago

Addressed by @Jared-Miller in PR #596. Closing this.