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

Implement Flood Fill Crafting Search #30444

Merged

Conversation

jmattspartacus
Copy link
Contributor

@jmattspartacus jmattspartacus commented May 12, 2019

Initial commit, recommit of PR30399's code because of git issues I was unable to overcome at the time.
Adds:
map::reachable_flood_steps()

How to use
Activate the crafting menu, and see if the crafting menu includes all things within 6 tiles that are not blocked off by impassible tiles, or spaces without floors

Summary
Summary: Features "Crafting inventory now uses flood fill search"

Makes the search for items for the crafting inventory use a flood fill for the search rather than only looking for items that are within 6 tiles in a straight line.

Purpose of change
When crafting, it's nice to have things in organized containers, but with the current search, it makes it so everything either has to be on the floor, or directly in sight. With a flood fill, it's possible to organize crates or shelves in rows, and provided they are within range, the item will be added to the crafting inventory.

Describe the solution
This flood fills tiles, starting from the player's location across the entire reachable area that is passable, and then if the location is found, the items in them are added to the crafting inventory.

Describe alternatives you've considered
I've considered hooking into the pathfinding code, but I'm not sure it's the right choice for this, as I don't understand all of the code there.

Comment:
I was unable to remove the changes from PR30399 that included files completely unrelated to the PR, I tried rebasing without any luck, and ended up making the situation worse.

@ifreund ifreund added [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones labels May 12, 2019
Copy link
Contributor

@OrenAudeles OrenAudeles left a comment

Choose a reason for hiding this comment

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

Small issues that are preventing successful compile.

src/map.h Outdated Show resolved Hide resolved
src/map.h Outdated Show resolved Hide resolved
@ifreund ifreund added the Code: Performance Performance boosting code (CPU, memory, etc.) label May 12, 2019
@jmattspartacus jmattspartacus force-pushed the Craft-Flood-Fill-Search-2 branch from c9c6311 to 0989cac Compare May 14, 2019 23:50
jmattspartacus and others added 2 commits May 15, 2019 22:51
Initial commit, recommit of PR30399's code because of git issues I was unable to overcome.
Adds:
map::reachable_flood_steps()
map::check_reachables()

Removed function qualification in header

Co-Authored-By: OrenAudeles <orenaudeles@gmail.com>

Remove function qualification in header

Co-Authored-By: OrenAudeles <orenaudeles@gmail.com>
map member functions: remove map:: qualifier to fix compile error

astyling

astyle check was failing on gorgon build, should fix that.

Remove unnecessary functions

Removing functions left over from the original implementation to prevent some code bloat where possible.
@jmattspartacus jmattspartacus force-pushed the Craft-Flood-Fill-Search-2 branch from 0989cac to 0d22446 Compare May 16, 2019 02:53
@kevingranade kevingranade merged commit e16cb64 into CleverRaven:master May 21, 2019
@ZhilkinSerg
Copy link
Contributor

Probably breaks Android build (see https://ci.narc.ro/job/Cataclysm-Android/8882/console):

0:27:35   [armeabi-v7a] Compile++ thumb: main <= inventory.cpp
10:27:35   In file included from /var/lib/jenkins/workspace/Cataclysm-Android/android/app/jni/src/inventory.cpp:1:
10:27:35   In file included from /var/lib/jenkins/workspace/Cataclysm-Android/android/app/jni/src/inventory.h:12:
10:27:35   /usr/local/android-sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/vector:2010:21: error: object of type 'tripoint_range::point_generator' cannot be assigned because its copy assignment operator is implicitly deleted
10:27:35                   __m = __first;
10:27:35                       ^
10:27:35   /var/lib/jenkins/workspace/Cataclysm-Android/android/app/jni/src/inventory.cpp:399:23: note: in instantiation of function template specialization 'std::__ndk1::vector<tripoint, std::__ndk1::allocator<tripoint> >::insert<tripoint_range::point_generator>' requested here
10:27:35           reachable_pts.insert( reachable_pts.begin(), in_radius.begin(), in_radius.end() );
10:27:35                         ^
10:27:35   /var/lib/jenkins/workspace/Cataclysm-Android/android/app/jni/src/map_iterator.h:20:39: note: copy assignment operator of 'point_generator' is implicitly deleted because field 'range' is of reference type 'const tripoint_range &'
10:27:35                   const tripoint_range &range;
10:27:35                                         ^
10:27:35   1 error generated.
10:27:35   make: *** [/var/lib/jenkins/workspace/Cataclysm-Android/android/app/build/intermediates/ndkBuild/release/obj/local/armeabi-v7a/objs/main/inventory.o] Error 1
10:27:35   make: *** Waiting for unfinished jobs....

@jniscior
Copy link

Breaks OS X builds:

clang++  -DRELEASE -DMACOSX -DGIT_VERSION -DOSX_SDL2_LIBS -DTILES -DBACKTRACE -DUSE_HOME_DIR -ffast-math -O3 -flto -Werror -Wall -Wextra -Woverloaded-virtual -Wpedantic    -fsigned-char -stdlib=libc++  -std=c++14 -MMD -MP -mmacosx-version-min=10.13 -D_THREAD_SAFE -I/usr/local/include/SDL2 -DSDL_SOUND -I/usr/local/include/SDL2 -D_THREAD_SAFE -I/usr/local/include -c src/inventory_ui.cpp -o obj/tiles/inventory_ui.o
In file included from src/inventory.cpp:1:
In file included from src/inventory.h:12:
/Applications/Xcode_10_2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/vector:2010:21: error: object of type 'tripoint_range::point_generator'
      cannot be assigned because its copy assignment operator is implicitly deleted
                __m = __first;
                    ^
src/inventory.cpp:399:23: note: in instantiation of function template specialization 'std::__1::vector<tripoint, std::__1::allocator<tripoint>
      >::insert<tripoint_range::point_generator>' requested here
        reachable_pts.insert( reachable_pts.begin(), in_radius.begin(), in_radius.end() );
                      ^
src/map_iterator.h:20:39: note: copy assignment operator of 'point_generator' is implicitly deleted because field 'range' is of reference type 'const tripoint_range &'
                const tripoint_range &range;
                                      ^
1 error generated.

ifreund added a commit to ifreund/Cataclysm-DDA that referenced this pull request May 21, 2019
in pr CleverRaven#30444 vector::insert was called with `tripoint_range::begin()`
which returns a `point_generator`.  This call is however invalid since
the called overload of `vector::insert()` requires the parameter to be
move assignable. `point_generator` is not implicitly copy assignable as
it contains a reference as a field and is therefore also not move
assignable.

This fixes the issue by replacing the `insert()` call with a range
based for loop.
@ifreund ifreund mentioned this pull request May 21, 2019
ifreund added a commit to ifreund/Cataclysm-DDA that referenced this pull request May 21, 2019
in pr CleverRaven#30444 vector::insert was called with `tripoint_range::begin()`
which returns a `point_generator`.  This call is however invalid since
the called overload of `vector::insert()` requires the parameter to be
move assignable. `point_generator` is not implicitly copy assignable as
it contains a reference as a field and is therefore also not move
assignable.

This fixes the issue by replacing the `insert()` call with a range
based for loop.
ifreund added a commit to ifreund/Cataclysm-DDA that referenced this pull request May 21, 2019
Code using inventories created with `inventory::form_from_map()`
expects the origin to be included, which is not always the case
after CleverRaven#30444.

This includes the crafting code, where the issues caused by this
are most obvious.
@jmattspartacus jmattspartacus deleted the Craft-Flood-Fill-Search-2 branch May 22, 2019 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants