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.

Sign up for GitHub

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

Jump to bottom

Filename shuffle #1007

Merged
merged 2 commits into from
Dec 12, 2022
Merged

Filename shuffle #1007

merged 2 commits into from
Dec 12, 2022

Conversation

Robadob
Copy link
Member

@Robadob Robadob commented Dec 8, 2022 •
edited
Loading

Requires fully CI build to check it works

  • Lots of files have moved into detail and need adding to the namespace.
  • Some test suites should now be moved to match the new file structure?
  • Remove CUDASimulation::getAgent() (it returns type detail::AgentInterface it was added for hacky reasons to access agent data on the device which are probably nolonger relevant.)

Closes #550

Base automatically changed from cmake-option-prefixing to master December 8, 2022 16:35
@Robadob Robadob force-pushed the filename_shuffle branch 3 times, most recently from 42465a6 to 8343b0c Compare December 12, 2022 09:01
@Robadob Robadob marked this pull request as ready for review December 12, 2022 09:03
@Robadob
Copy link
Member Author

Robadob commented Dec 12, 2022

Just building test suite locally, but I think this is ready to merge.

@Robadob Robadob requested a review from ptheywood December 12, 2022 09:03
@Robadob Robadob force-pushed the filename_shuffle branch 4 times, most recently from c9fedcd to 83a9577 Compare December 12, 2022 10:05
@Robadob
Copy link
Member Author

Robadob commented Dec 12, 2022 •
edited
Loading

Windows/Debug/Seatbelts=ON test suite built locally.

[----------] Global test environment tear-down
[==========] 1063 tests from 82 test suites ran. (379961 ms total)
[  PASSED  ] 1063 tests.

  YOU HAVE 53 DISABLED TESTS


===================================== 648 passed, 11 skipped in 349.58s (0:05:49) =====================================

(After correcting an issue in JitifyCache::getIncludeDir() that had been broken whilst updating include dirs with find-replace.)

@Robadob Robadob force-pushed the filename_shuffle branch 2 times, most recently from 1be488e to 2ff60e5 Compare December 12, 2022 10:52
@Robadob
Breaking Change: Files within include/src/tests were reorganised into…
1d65452 
… a more logical structure.

This includes a number of classes being added to the detail namespace.
@Robadob
Breaking Change: Removed CUDASimulation::getAgent()/getCUDAAgent()/ge…
479afe4 
…tCUDAMessage()

These features were not intended for release, users should not have direct access to internal objects (detail namespace). Roughly equivalent functionality is available via DeviceAgentVector within the HostAPI.

fixup
ptheywood
ptheywood approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@ptheywood ptheywood left a comment •
edited
Loading

Choose a reason for hiding this comment

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

File moves look good, Tests and pytest all pass under linux.

Should be fine to merge once the pytests are restructured similar to the cxx tests.

README.md Outdated Show resolved Hide resolved
@Robadob Robadob merged commit 313e1dc into master Dec 12, 2022
@Robadob Robadob deleted the filename_shuffle branch December 12, 2022 11:52
@Robadob Robadob mentioned this pull request Dec 12, 2022
Closed
Robadob added a commit that referenced this pull request Dec 12, 2022
@Robadob
BugFix: FLAMEGPU_ENABLE_GLM was broken
31abfe6 
This was broken by #1007 earlier today

Also removed a leftover file from the same PR, and improved the syntax of a few includes.

Closes #1011
@Robadob Robadob mentioned this pull request Dec 12, 2022
Merged
Robadob added a commit that referenced this pull request Dec 12, 2022
@Robadob
BugFix: FLAMEGPU_ENABLE_GLM was broken
7878870 
This was broken by #1007 earlier today

Also removed a leftover file from the same PR, and improved the syntax of a few includes.

Closes #1011
@ptheywood ptheywood mentioned this pull request Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptheywood ptheywood ptheywood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Class / Namespaces to Rename / Move
2 participants
@Robadob @ptheywood