Expected/Desired Behavior or Experience:

CMake diagnostic messages should appear only in a neutral/common color and not look alarming.

Actual Behavior:

CMake diagnostic messages are output in some contexts in a red color.

Steps to Reproduce the Actual Behavior:

Run Configure in CMake GUI and notice the resulting output.

Expected/Desired Behavior or Experience:

The build system skips prs where no source code files were modified.

Actual Behavior:

I just noticed when I submitted pr #54 that the system is running the build tests (and I assume it's going to run coveralls and the benchmark tests afterward). The only change I made was to a documentation (.md) file. I know the consequences for us (the maintainers) are minimal, but this seems like a poor use of public resources.

Steps to Reproduce the Actual Behavior:

Submit a pr with only documentation changes.

Expected/Desired Behavior or Experience:

I would like to be able to build using CMake on Windows so I can address the unit test linking issue #1.

Actual Behavior:

CMake returns with the following errors (after specifying the glew library location correctly):

PLAYRHO_REALNUM_TYPE=float
CMake Error at C:/Program Files (x86)/CMake/share/cmake-3.9/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
Call Stack (most recent call first):
  C:/Program Files (x86)/CMake/share/cmake-3.9/Modules/FindPackageHandleStandardArgs.cmake:377 (_FPHSA_FAILURE_MESSAGE)
  C:/Program Files (x86)/CMake/share/cmake-3.9/Modules/FindPkgConfig.cmake:36 (find_package_handle_standard_args)
  Testbed/CMakeLists.txt:4 (find_package)

Steps to Reproduce the Actual Behavior:

I found this behavior while following the old CMake setup instructions. I've installed pkg-config using information found in the answers to this question (along with the necessary .dll files, verifying that the program runs), and adding PKG_CONFIG_EXECUTABLE=C:\path\to\pkg-config.exe in the CMake GUI. When I configure, it deletes the entry and reports the same error as above.

I built the project on Windows 7 using the provided solution file (VS Community 2017). Everything worked pretty smooth. I can pull the boxes around, etc... except I don't see any text onscreen when I run the testbed as shown in the image below. I think the buttons and sliders are working properly (the very bottom right button exits the program at very least).

I also have a few other constructive comments about the project in general that don't really deserve to have issues opened for each of them. Is there another place the project can be discussed? Alternatively, feel free to send me an email!

Expected/Desired Behavior or Experience:

It's okay for GetMaxSeparation4x4 to return different values so long as they're still functionally equivalent enough. What enough is needs to be better understood though so that either the relevant unit test code can be update to accept the results, or the GetMaxSeparation4x4 implementation needs to be corrected. I.e. either the unit test needs to be recognized as broken or the GetMaxSeparation4x4 needs to be. And the broken one fixed.

Actual Behavior:

GetMaxSeparation4x4 returns different values than GetMaxSeparation which the unit test World.TilesComesToRest notices as different StepStats performance characteristics.

Steps to Reproduce the Actual Behavior:

Run the World unit tests.

Undefined behaviour on archlinux x86_64.
Built with cmake -DPLAYRHO_BUILD_TESTBED=On -DPLAYRHO_BUILD_UNIT_TESTS=On -DPLAYRHO_INSTALL=On .. && make -j8

Expected/Desired Behavior or Experience:

All tests has to complete fine.

Actual Behavior:

Tests can't even be completed.

Steps to Reproduce the Actual Behavior:

I just built project and started to run tests. I run them for 3 times to get stack smashing.

Expected/Desired Behavior or Experience:

API Documentation is up to date to the API made available for the beta launched code. An automated updating capability of the documentation would be nice but not at the expense of weakening branch security.

Actual Behavior:

The API documentation is behind the API at present. The API is still changing and the API documentation is generated manually using Doxygen in the gh-pages branch of the repository.

Expected/Desired Behavior or Experience:

The Settings.hpp and Settings.cpp files don't have non-settings related code in it for dynamic memory management.

Actual Behavior:

Functions like Alloc and Free are defined in the Settings files.

Steps to Reproduce the Actual Behavior:

Visual inspection.

Expected/Desired Behavior or Experience:

Speed up the engine with additional compiler optimizations without unit tests failing nor noticeably undesirable behavior.

Actual Behavior:

Release mode builds don't optimize to the point of possibly breaking strict standards compliance nor is the code written to deal with being compiled by clang options like -fno-signed-zeros -freciprocal-math -ffp-contract=fast -menable-unsafe-fp-math -menable-no-nans -menable-no-infs. This results in multiple unit tests which fail.

Steps to Reproduce the Actual Behavior:

Add the -Ofast compiler option, rebuild with it, and run the unit tests.

I've thought over the years that not all joints were created equal, and some joints that people never used were unstable or something like that. After playing around with the testbed, love2d, etc... I realized that there were really neat behaviors created by the WeldJoint among others which I will open other issues for. It struck me, that the WeldJoint is functionally an angular analogue of the DistanceJoint. I discuss this in depth with pictures and gifs in this GDSE answer, so I won't repeat it here.

I think it's pretty unlikely for box2d to adopt this change. It would also be a poor choice as it now has so many flavors, bindings, and re-implementations across the net that would need to update. For PlayRho, however, a change like this would be very possible in its current state, and would communicate the purpose of the joint more clearly.

What does everyone else think about renaming the WeldJoint to AngleJoint? Is there another name that would fit better?

Expected/Desired Behavior or Experience:

The style of the API documentation is consistent with the markdown/web style at the IO web pages.

Actual Behavior:

The style of the API documentation is basically the default Doxygen HTML output style.

Steps to Reproduce the Actual Behavior:

To see the difference in style in a side-by-side manner, open one browser window to IO web pages, and open another browser window to API documentation at the same time and compare the styles.

Expected/Desired Behavior or Experience:

There should be no spelling mistakes in C++ comments especially since many of the comments are used in Doxygen documentation output. Need a command line tool for checking for spelling mistakes. This tool should be both manually and automatically runnable.

Actual Behavior:

There's currently no command line tool in place to check for spelling mistakes. Not manually nor automatically.

Steps to Reproduce the Actual Behavior:

Make a pull request with C++ code in it that has a comment with some words misspelled and notice that the misspellings don't get flagged by the PR mechanism.

Expected/Desired Behavior or Experience:

gtest-printers builds as expected.

Actual Behavior:

gtest-printers fails with errors:

C2338 <hash_map> is deprecated and will be REMOVED. Please use <unordered_map>. You can define _SILENCE_STDEXT_HASH_DEPRECATION_WARNINGS to acknowledge that you have received this warning. gtest-printers_test C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\include\hash_map 18

C2338 <hash_set> is deprecated and will be REMOVED. Please use <unordered_set>. You can define _SILENCE_STDEXT_HASH_DEPRECATION_WARNINGS to acknowledge that you have received this warning. gtest-printers_test C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\include\hash_set 18

Steps to Reproduce the Actual Behavior:

Build the unit tests with Windows 10, VS 2017 project generated using CMake 3.9.0 . Not sure if this is a Windows-specific issue.

Redo the JointDef class hierarchy to provide a builder interface via static polymorphism. While at it, shrink the number of parameters to JointDef constructors to bare minimums; i.e. get rid of defaulted parameters in favor of requiring use of builder methods.

I built PlayRho as -DCMAKE_BUILD_TYPE=Release. But StackAllocator.slower_than_mallocfree test still failed.

Expected/Desired Behavior or Experience:

Test has to be completed fine.

Actual Behavior:

Test is failed.

Steps to Reproduce the Actual Behavior:

I just built project with cmake -DPLAYRHO_BUILD_UNIT_TESTS=On -DPLAYRHO_BUILD_TESTBED=On -DPLAYRHO_INSTALL=On -DCMAKE_BUILD_TYPE=Release .. && make -j8 and just run UnitTests executable. And the only test that failed for now is slower_than_mallocfree with message:

PlayRho/UnitTests/StackAllocator.cpp:95: Failure
Expected: (elapsed_secs_custom.count()) > (elapsed_secs_malloc.count()), actual: 1.08923 vs 4.47813

Expected/Desired Behavior or Experience:

The following two Joint methods would replace the current GetReaction* methods:

• Momentum2D Joint::GetLinearReaction() const.
• AngularMomentum Joint::GetAngularReaction() const.

This is along the path of reducing classes to their essences and increasing encapsulation. This will help with reducing joint class complexity and refactoring them to be closer to the modeling by the Contact, PositionConstraint, and VelocityConstraint classes.

Actual Behavior:

The Joint base class's current method signatures are:

• Force2D Joint::GetReactionForce(Frequency inv_dt) const.
• Torque Joint::GetReactionTorque(Frequency inv_dt) const.

Steps to Reproduce the Actual Behavior:

Visual inspection.

If build PlayRho with tests and then install it, PlayRho will install libgtest library to system.
Proof:

-- Installing: /usr/local/lib/libgmock.a
-- Installing: /usr/local/lib/libgmock_main.a
-- Installing: /usr/local/include/gmock
-- Installing: /usr/local/include/gmock/internal
-- Installing: /usr/local/include/gmock/internal/gmock-generated-internal-utils.h.pump
-- Installing: /usr/local/include/gmock/internal/gmock-port.h
-- Installing: /usr/local/include/gmock/internal/custom
-- Installing: /usr/local/include/gmock/internal/custom/gmock-port.h
-- Installing: /usr/local/include/gmock/internal/custom/gmock-generated-actions.h.pump
-- Installing: /usr/local/include/gmock/internal/custom/gmock-generated-actions.h
-- Installing: /usr/local/include/gmock/internal/custom/gmock-matchers.h
-- Installing: /usr/local/include/gmock/internal/gmock-generated-internal-utils.h
-- Installing: /usr/local/include/gmock/internal/gmock-internal-utils.h
-- Installing: /usr/local/include/gmock/gmock-actions.h
-- Installing: /usr/local/include/gmock/gmock-generated-matchers.h.pump
-- Installing: /usr/local/include/gmock/gmock-generated-matchers.h
-- Installing: /usr/local/include/gmock/gmock-more-actions.h
-- Installing: /usr/local/include/gmock/gmock-generated-actions.h.pump
-- Installing: /usr/local/include/gmock/gmock-generated-actions.h
-- Installing: /usr/local/include/gmock/gmock-more-matchers.h
-- Installing: /usr/local/include/gmock/gmock-cardinalities.h
-- Installing: /usr/local/include/gmock/gmock-generated-nice-strict.h
-- Installing: /usr/local/include/gmock/gmock-spec-builders.h
-- Installing: /usr/local/include/gmock/gmock-matchers.h
-- Installing: /usr/local/include/gmock/gmock-generated-nice-strict.h.pump
-- Installing: /usr/local/include/gmock/gmock-generated-function-mockers.h
-- Installing: /usr/local/include/gmock/gmock.h
-- Installing: /usr/local/include/gmock/gmock-generated-function-mockers.h.pump
-- Installing: /usr/local/lib/libgtest.a
-- Installing: /usr/local/lib/libgtest_main.a
-- Installing: /usr/local/include/gtest
-- Installing: /usr/local/include/gtest/internal
-- Installing: /usr/local/include/gtest/internal/gtest-tuple.h
-- Installing: /usr/local/include/gtest/internal/gtest-filepath.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util.h
-- Installing: /usr/local/include/gtest/internal/gtest-type-util.h.pump
-- Installing: /usr/local/include/gtest/internal/gtest-port-arch.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util-generated.h
-- Installing: /usr/local/include/gtest/internal/gtest-type-util.h
-- Installing: /usr/local/include/gtest/internal/custom
-- Installing: /usr/local/include/gtest/internal/custom/gtest.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-port.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-printers.h
-- Installing: /usr/local/include/gtest/internal/gtest-port.h
-- Installing: /usr/local/include/gtest/internal/gtest-string.h
-- Installing: /usr/local/include/gtest/internal/gtest-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-tuple.h.pump
-- Installing: /usr/local/include/gtest/internal/gtest-death-test-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-linked_ptr.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util-generated.h.pump
-- Installing: /usr/local/include/gtest/gtest-message.h
-- Installing: /usr/local/include/gtest/gtest-test-part.h
-- Installing: /usr/local/include/gtest/gtest.h
-- Installing: /usr/local/include/gtest/gtest-typed-test.h
-- Installing: /usr/local/include/gtest/gtest-param-test.h.pump
-- Installing: /usr/local/include/gtest/gtest_prod.h
-- Installing: /usr/local/include/gtest/gtest-printers.h
-- Installing: /usr/local/include/gtest/gtest-spi.h
-- Installing: /usr/local/include/gtest/gtest_pred_impl.h
-- Installing: /usr/local/include/gtest/gtest-death-test.h
-- Installing: /usr/local/include/gtest/gtest-param-test.h

Expected/Desired Behavior or Experience:

make install command installs only PlayRho library.

Actual Behavior:

make install installs PlayRho library and libgtest.

Steps to Reproduce the Actual Behavior:

Configure PlayRho with -DPLAYRHO_INSTALL=On -DPLAYRHO_BUILD_UNIT_TESTS=On and perform make install.

Let's use this issue for any conversation regarding SIMD/MIMD enhancements.

Expected/Desired Behavior or Experience:

CMake-generated VS solution builds without issue

Actual Behavior:

Compiler generates warnings 'Unrecognized flag "-O3"' when building.

I noticed there are a few lines in PlayRho/CMakeLists.txt:

# Tell C++ compiler to optimize release builds for speed.
# In clang++, the optimize for speed flag is '-Ot'. This option isn't supported on g++
# however and it'd be nice to use an option that works for both compilers. So use '-O3'.
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3" )

Omitting the -O3 eliminates the warnings.

Can I create a clause so that for the Visual Studio configurations this flag is omitted?

I'm trying to install library to system library path, but at the end of installation I'm getting error message:

CMake Error at PlayRho/cmake_install.cmake:201 (file):
  file INSTALL cannot find
  "/home/megaxela/Development/Build/PlayRho/PlayRho/UsePlayRho.cmake".
Call Stack (most recent call first):
  cmake_install.cmake:37 (include)


make: *** [Makefile:74: install] Error 1

Build files was generated with cmake 3.8.2 with command cmake -DPLAYRHO_BUILD_UNIT_TESTS=On -DPLAYRHO_BUILD_TESTBED=On -DPLAYRHO_INSTALL=On -DCMAKE_BUILD_TYPE=Release ..

Expected/Desired Behavior or Experience:

Library has to be installed fine.

Actual Behavior:

Library is installing fine and as for me it's even working fine.

Steps to Reproduce the Actual Behavior:

Build project with -DPLAYRHO_INSTALL=On key on configuration step. Then on make install step it'll fail with recently provided error.

From @louis-langholtz on January 7, 2017 22:42

Here's a screenshot showing the current state and issue:

Copied from original issue: louis-langholtz/Box2D#3

Pick a benchmarking library like Google Benchmark for this.

Migrated here from my Box2D fork issue #24.

At last check, the project's library and the unit tests all build on Windows. But the unit tests aren't linking with everything it needs.

Mismatch of link options. Part built with MDd and other with MTd if memory serves me.

Incidentally, I've updated the MSVS solution and project files and gotten the Testbed and its dependents to all build and link and run under MS VS2017.

Expected/Desired Behavior or Experience:

Change default settings to be closer to what users likely expect from Erin's Box2D. Make it easier for users to do apples-to-apples comparisons between how Erin's Box2D library behaves and how the PlayRho library behaves (at least by default). Having similar defaults should also help isolate differences that are significant or problematic and needing of more attention (whether by documentation or by fixes).

Actual Behavior:

Things like a smaller linear slop value significantly increases computation needed before simulations come to rest. These differences may confuse users. For instance PlayRho could appear to be slower when instead it's just doing more calculations. There may be speed differences but there's no need to create these unnecessarily by default.

This is a minimum bar that I'd like to exceed.

Expected/Desired Behavior or Experience:

An exception is thrown if a create method like World::CreateBody (or World::CreateJoint etc) has an issue for which it can't return a pointer to the new object.

Actual Behavior:

Depending on the circumstance, sometimes a nullptr is returned.

Steps to Reproduce the Actual Behavior:

Create MaxBodies body object using the World::CreateBody method. Then create one more body. The return value is nullptr instead of getting an exception.

Discussion

While I'm of the opinion that returning nullptr isn't the worst solution in error like situations and may even be preferred in some contexts, I'm concerned that user code would need to check for nullptr or likely crash, and even if the user checked for nullptr, that doesn't exactly give user code much to go on for why the operation failed. So throwing an exception seems more likely to be an informative way to deal with this.

Expected/Desired Behavior or Experience:

AppVeyor CI does CI for Release configuration as well as Debug configuration.

Actual Behavior:

AppVeyor CI only does Debug configuration CI.

Steps to Reproduce the Actual Behavior:

Inspect a log prior to this issue being dealt with like this one. Notice that the CONSOLE tab log shows Debug for the building solution configuration. There's no build log for that time which builds for the Release configuration.

I'll admit it, one of the things that sways my opinion of any online software project is the presence of a sleek logo. In some ways it's vein, but a good logo can also give a great first impression of the library, before the visitor reads a single line of text.

Here's Box2d's logo:

Fair use, Link

It get's the point across in a hurry, but it's not very elegant. The lines in the top right actually draw my attention off the top right. It's also a little rough around the edges (literally, with Github dark theme you can see the white borders around each of the objects).

What's are the coolest features of the PlayRho library? Do any of them fit into a logo?

GitHub user JesseTG made the suggestion in the past of writing a pull request template containing a Contributor License Agreement (CLA) that by people submitting pull requests they agree to abide by the CLA. I think this is a great suggestion and would love to see it done.

Expected/Desired Behavior or Experience:

Testbed builds successfully from CMake-generated build and provided VS solution.

Actual Behavior:

Build fails with compiler errors C2662 in both Testbed/Tests/HeavyOnLight.cpp and similar code within UnitTests/World.cpp :

Error	C2662	'playrho::BodyDef &playrho::BodyDef::UseLocation(playrho::Length2D) noexcept': cannot convert 'this' pointer from 'const playrho::BodyDef' to 'playrho::BodyDef &'	Testbed	c:\path\to\playrho\testbed\Tests\HeavyOnLight.hpp	34

Line 34 is:

        const auto upperBodyDef = BodyDef{bd}.UseLocation(Vec2(0.0f, 6.0f) * Meter);

Similar errors occur where lowerBodyDef, smallerDiscConf, and biggerDiscConf. There are similar compiler errors in World.cpp. These errors cascade into other compiler errors, so I'll post those as issues if resolving these does not fix them.

I'm not entirely sure, but I think the today's visual studio update may have something to do with this error.

Steps to Reproduce the Actual Behavior:

Build the CMake-generated VS2017 project or the supplied project.

Expected/Desired Behavior or Experience:

To have a Benchmark specific README.md file.

Actual Behavior:

None present at present.

Expected/Desired Behavior or Experience:

A user guide would be available from https://louis-langholtz.github.io/PlayRho/ that basically provides the information found in Documentation/manual.docx. This should not be in DOCX format but a web page format like HTML (preferably from markdown).

Actual Behavior:

Documentation/manual.docx currently applies to Erin Catto's Box2D from 2.3.2. While many of the concepts may be the same, most - if not all - of the API names have changed. Also much of the interface's inputs, outputs, and I/O parameters have changed.

Steps to Reproduce the Actual Behavior:

See Documentation/manual.docx.

Expected/Desired Behavior or Experience:

I'd like to have a CMakeLists.txt file in the Benchmark folder to help with building the Benchmark application on general platforms.

Actual Behavior:

No CMakeLists.txt file exists currently in this folder.

Expected/Desired Behavior or Experience:

Closer compile-time compatibility with C++17-styled std::array elements. Generic data access via templates to the underlying elements.

Actual Behavior:

Most places in the current code base use .x and .y values as expected as horizontal and vertical values. Some places however (like in the Joint implementation code) overload these semantic notions (of the horizontal and the vertical) with other notions. Like treating them as array elements and offering indexed accessors (see float32& b2Vec2::operator()(int32 i); for example). This overloading has been noted before by users of Erin's Box2D.

Discussion

When I forked Box2D from Erin's codebase which had float32& b2Vec2::operator()(int32 i); in it, I changed this method to instead overload the indexing operator for this functionality and to use a switch statement to access .x and .y components. This served the purpose of using the [] operator which seems more conventional to me for indexed access to data, and served the purpose of avoiding relying the memory layout being compatible. For the sake of discussion, I'll call the former concept a structure of fields and the latter concept an array of elements.

Since then, I've become less fond of the conceptual bias of these structures being collections of fields and more fond of the bias towards being arrays of elements.

For one thing, while .x and .y are just names and access is 1 less character to type than say [0] and [1], I like ascribing meaning to names and enforcing those when possible.

PlayRho could have the existing Vec2 named type being a type alias to Vector2D<Real> like it does now with x and y fields and drop the indexed access support, and gain a separate type like Vec2A which would have indexed access support but not the x and y fields. Alternatively, PlayRho could just drop the .x and .y field access support, and just use an array of elements.

With the former stance, the meanings I see are:

1. Components are instance of their type.
2. Components are intended to be contiguous in memory.2.
3. Components are horizontal and vertical values.

With the latter stance, the meanings I see are:

1. Components are instances of their type (still).
2. Components are intended to be contiguous in memory.

So the latter stance loses a meaning. Sort of. The former stance is kind of like a union though in not being so C++ friendly as say C++17 std::variant, because the former stance doesn't ensure that a Vec2 set as an x and y aggregate isn't later accessed as an array nor vice-versa. In other words, in the current implementation, it doesn't enforce three intentions; just two of them. So I ask, why not drop the charade and just go with the latter stance that doesn't impede components [0] and [1] from being used as x and y concepts nor from being used as a pair of impulse concepts either (as is done in the solver code).

For a second and perhaps more important thing however, while .x and .y access is 1 less character to type than say [0] and [1], it seems to me that pure array styled access lends itself more directly to generic templated code. Instead of having code in Vector2D and Vector3D, we could have a base Vector template structure that Vector2D and Vector3D were simply alias templates for. Some code for Vector could then be shared for Vector2D and Vector3D like support for begin and end iterator methods.

This second attribute also seems to have beneficial implications for Mat22 and Mat33 structures which currently aren't templated. But I'd like them to be templates so that we can say have a Matrix22<Mass> for instance which is actually how many joints use Mat22 now but with the types stripped down to base Real values.

This second attribute also seems to have beneficial implications for things like a more natural application of std::inner_product and other standard library functionality.

Would be awesome to have a website that provided benchmarking trend analysis. Something like coveralls.io that showed the historical trend but for performance metrics (instead of unit test code coverage metrics).

Does such a website/service even exist?

From @louis-langholtz on April 18, 2017 18:50

Generally speaking, I want this project to be open to taking pull requests. I'm not certain how to answer this more specifically however.

A problem I can foresee with taking pull requests, is that this project is in an alpha state in my opinion. I have some major refactoring work that I intend to be pushing to the project where fundamental designs change. I do try and ensure that nothing gets pushed that breaks compiles though.

Comparatively speaking, I think that I'm more comfortable right now considering pull requests for the Testbed or Unit Test portions of the code than the main library code.

Note that I won't take any pull requests that don't appear to follow proper guidelines for appropriately handling copyrights and licensing. If you modify files that you want to be considered for pulling, it's my understanding that the appropriate handling means that you'll have to maintain the project's license and you'll have to note in the header that you've modified the code and how you can be contacted.

IANAL however and I am open to feedback and refinement of this.

Copied from original issue: louis-langholtz/Box2D#20

Expected/Desired Behavior or Experience:

AppVeyor CI builds both the Debug and Release configurations for both the x64 and x86/Win32 platforms.

Actual Behavior:

AppVeyor CI builds both the Debug and Release configurations but for only the x64 platform.

Steps to Reproduce the Actual Behavior:

Inspect an AppVeyor build prior to this issue getting done like this one. Note that there are two jobs shown named "Configuration: Debug" and "Configuration: Release". Nothing is shown for multiple platforms though.

Expected/Desired Behavior or Experience:

The Benchmark application is part of CI for at least one target.

Actual Behavior:

PRs with code changes for the Benchmark application can pass all checks even with invalid code since the Benchmark application isn't compiled/built for even one target as part of the CI system.

Steps to Reproduce the Actual Behavior:

Submit a PR with changes to Benchmark code with invalid C++14 code.

Hi Louis,

I came by this repo through this answer. It looks like a well maintained project for the most part, good job! Maybe I'll try building soon.

I did notice, however that the links to the Changes Document, Building Document, and API Pages are all broken as of now.

Expected/Desired Behavior or Experience:

Rename the Math.hpp functions to consistently align with the convention of starting with a capital letter and capitalizing the first letter of successive words (instead of separating them with underscores).

Actual Behavior:

We have the following non-compliant function names in Math.hpp:

• round
• almost_zero
• almost_equal

Steps to Reproduce the Actual Behavior:

Visually review the current Math.hpp file.

A source code style guide is needed.

Among other things, it should require formatting to be using 4-spaces instead of tabs. I've put .editorconfig EditorConfig files into the source code trees now to help with this. Seems like a clang format file would be ideal for this.

From @Genbox on April 2, 2017 11:46

Hi Louis,

I've seen you active around the Box2D project, and it seems like you are keen to improve it and add features. I'm working on Velcro Physics, which is a Box2D port in C#, but extended with many new features such as path generators, better performance, concave polygons etc.

I'm interested in your port as it seems like you made some improvements and new features, but I have a hard time going through hundreds of files and look for them, so could we have a discussion here on what improvements you have made?

Copied from original issue: louis-langholtz/Box2D#12

Expected/Desired Behavior or Experience:

Behavior like C++17 std::optional that's practically drop-in replaceable till C++17 is more widely available. Add a playrho::Optional that's a using alias for the new playrho::OptionalValue.

Where-ever code currently uses classes like RayCastOutput, they'd instead use code like playrho::Optional<RayCastHit>.

Actual Behavior:

Classes like RayCastOutput, Manifold, effectively employ a Boolean that has to be used to determine if the remaining values are meaningful or not. The intention of the data being optional isn't expressed as explicitly as using the Optional alias.

Steps to Reproduce the Actual Behavior:

See the current RayCastOutput code in RayCastOutput.hpp.

Candidate Functions

• RayCast functions/methods currently returning RayCastOutput. Eliminates need for RayCastOutput::hit parameter.
• CollideShapes (return Manifold). Eliminates need for Manifold::Type::e_unset.
• GetUnitVector (returning UnitVec2). Eliminates the fallback parameter.
• UnitVec2::Get (returning UnitVec2). Eliminates the fallback parameter.
• BlockSolveNormalCase1.
• BlockSolveNormalCase2.
• BlockSolveNormalCase3.
• BlockSolveNormalCase4.

Refactoring these often eliminates uses of NaN as sentinel.

Add a new console application target that can be built with the chosen benchmarking library and the physics library to benchmark code of interest.

Expected/Desired Behavior or Experience:

Using nothing anymore from the Box2D repo that this repo originated from.

Actual Behavior:

$ git config --get-regex submodule*
submodule.Box2D/UnitTests/googletest.url https://github.com/google/googletest

Severity of Problem

Unclear. Seemingly low to none. But it might be confusing for new clones or new users.

From @louis-langholtz on January 12, 2017 15:12

Is it worth supporting concave shapes? Can all concave shapes already be supported via multiple fixtures having convex polygons? Is there a more efficient way to do concave shapes?

I believe that at least part of the problem of supporting concave polygons is supporting inside corners. This seems to have some implementation already in chain/edge shapes.

If nothing else, is there an algorithm for constructing any concave polygon out of convex polygons? Can this be conveyed more easily to users?

Copied from original issue: louis-langholtz/Box2D#7

Expected/Desired Behavior or Experience:

Users don't get confused.

Actual Behavior:

Enabling the display of contact information (contact points, normals, impulses) looks odd given where they appear in a dynamic simulation like the Tumbler demo. Here's an example of this apparent misalignment:

This is not a bug however. Just an artifact of when PlayRho updates contact related information.

I can modify Erin's Box2D to only update contact information the same way and then see a similar display:

Steps to Reproduce the Actual Behavior:

Run the Testbed Tumbler demo with the contact information displayed. Zoom in to a busy region with lots of contact info being shown. Pause the simulation. Dots don't appear located where I'd expect them.

Initial Recommendations

Not sure what's the best to do about this.

Documenting that the dots shown are the locations impulses are applied, while locations of bodies are the results of applying those impulses and any accelerations (like gravity) is one possibility.

Pulling the contact updating back outside the context of the step simulating any time is another option. This may appear less confusing to someone that's used Erin's Box2D before but then the display loses the ability to show where impulses were applied and the results; instead showing where impulses will next be applied.

Which Is preferable or less confusing to someone completely new to a physics engine? Is there another way to address this oddity? I'd like to hear from others on this.

Expected/Desired Behavior or Experience:

Version code doesn't co-mingle with Settings code.

Actual Behavior:

Version code is in the same file as settings (in Settings.hpp and Settings.cpp.

Steps to Reproduce the Actual Behavior:

Visually inspect Settings.hpp and Settings.cpp files.

Expected/Desired Behavior or Experience:

Something like the following where the contact info for a selected shape is highlighted over the non-selected shapes (along with highlighting the selected shape) :

Actual Behavior:

Like the above picture but without the selected shape's contact points, normals, and impulses highlighted.

Steps to Reproduce the Actual Behavior:

Run Testbed demo with Contact Points, Contact Normals, Contact Impulses, and Friction Impulses toggles selected.

Related Issues

This may help with diagnosing problems like the Tumbler squishiness described in issue #10 .

After the discussion in issue #25, I've decided to start a new thread for the potential re-naming of many of the box2d joints. As some Joints could be interpreted in similar ways (e.g. RevoluteJoint and WeldJoint) it makes sense think about the entire set of names and how they might overlap or be confused with one another. I created a Classeur markdown document with a table containing .gifs` of most of the joints, documentation text from Box2D and Unity, as well as my thoughts on new names.

I'm not quite finished yet, but most of my thoughts are there. Please comment, I want to end up with names that work for everyone!