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

Allow professions to start with a nearby vehicle - trucker profession #37314

Merged
merged 6 commits into from Mar 6, 2020
Merged

Allow professions to start with a nearby vehicle - trucker profession #37314

merged 6 commits into from Mar 6, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jan 23, 2020

Summary

SUMMARY: Features "Allow professions to start with a nearby vehicle - trucker profession"

Purpose of change

Allow professions to start with a nearby vehicle - trucker profession

Describe the solution

Define a vproto_id in the profession json.
This will get loaded on new game, it will try and place the vehicle on a nearby road ( some scenarios may make this difficult ) - it will always place on z-level 0, no matter where the player starts.
It will try 10 times in an expanding radius, it wont merge with other vehicles or cause wreckage.
It will then mark it on the overmap as if oyu had selected "remember this vehicles position".

Added a trucker profession as a basic showcase, starts with a semi truck nearby.

Describe alternatives you've considered

N/A

Testing

Made new game, chose trucker profession, tried around 10 times, in each case, the truck was rarely more than like 5 -10 omts away, on a road.
This was in the shelter start.

Additional context

N/A

@ghost ghost changed the title [WIP]Allow professions to start with a nearby vehicle - trucker profession Allow professions to start with a nearby vehicle - trucker profession Jan 23, 2020
@ZhilkinSerg ZhilkinSerg added 0.E Feature Freeze [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies labels Jan 23, 2020
data/json/professions.json Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
doc/JSON_INFO.md Outdated Show resolved Hide resolved
src/string_id_null_ids.cpp Outdated Show resolved Hide resolved
src/profession.cpp Outdated Show resolved Hide resolved
src/newcharacter.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@codemime codemime left a comment

Choose a reason for hiding this comment

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

The idea is awesome, but the implementation needs some polishing.

src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/overmapbuffer.cpp Outdated Show resolved Hide resolved
@Amoebka
Copy link
Contributor

Amoebka commented Jan 27, 2020

The fact that it can just give up and not spawn the vehicle if it can't find a road makes the profession way too random in some cases. A number of starting locations aren't guaranteed to spawn anywhere near roads, and some worldgen settings generate worlds without any roads at all. This can be confusing to players - the description says you have a truck but you don't.

Ideally there should be a way to blacklist professions based on world settings and blacklist starting locations based on professions for this to work properly. Alternatively, it should spawn the truck on an empty field tile if it can't find a road.

@ghost
Copy link
Author

ghost commented Jan 27, 2020

The fact that it can just give up and not spawn the vehicle if it can't find a road makes the profession way too random in some cases. A number of starting locations aren't guaranteed to spawn anywhere near roads, and some worldgen settings generate worlds without any roads at all. This can be confusing to players - the description says you have a truck but you don't.

Ideally there should be a way to blacklist professions based on world settings and blacklist starting locations based on professions for this to work properly. Alternatively, it should spawn the truck on an empty field tile if it can't find a road.

spawning in a field, yeah.. this could end up with the vehicle spawning totally enclosed by forest or something, and "beached", ,but I guess thats better than no vehicle at all.

I suppose yes, if you spawn on a lake island or something, this could be impossible to spawn the vehicle, and also what if a profession selects a boat to start with?

I will think on some elegant ( hopefully ) solutions to this, ill put this into WIP ( its in feature freeze anyway )

Ill think of some more fallback options to make sure peeps get their starting vehicle.

@ghost ghost changed the title Allow professions to start with a nearby vehicle - trucker profession [WIP]Allow professions to start with a nearby vehicle - trucker profession Jan 27, 2020
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.h Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Jan 29, 2020

Added fallback options depending on whether the vehicle is a boat or car or both, so if road fails, it will go to fields, and for boats it will search for rivers and lakes.

@ghost ghost changed the title [WIP]Allow professions to start with a nearby vehicle - trucker profession Allow professions to start with a nearby vehicle - trucker profession Jan 29, 2020
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
@ghost ghost requested a review from codemime February 10, 2020 10:16
src/game.h Outdated Show resolved Hide resolved
src/player.h Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/overmapbuffer.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@codemime codemime left a comment

Choose a reason for hiding this comment

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

Almost there. The comments below are mostly nitpicking (nice to have done, but not strictly necessary). The code is reasonably good as it is. Didn't test the behavior though.

src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Feb 17, 2020

Thanks for the thorough review, tested and working after changes.

@ZhilkinSerg ZhilkinSerg changed the base branch from master to dev March 6, 2020 11:34
@ZhilkinSerg ZhilkinSerg merged commit b07fe1e into CleverRaven:dev Mar 6, 2020
@@ -26,6 +26,7 @@ MAKE_NULL_ID( faction, "NULL" )
MAKE_NULL_ID( ammunition_type, "NULL" )
MAKE_NULL_ID( vpart_info, "null" )
MAKE_NULL_ID( emit, "null" )
MAKE_NULL_ID( vehicle_prototype, "null" )
Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently it should be MAKE_NULL_ID2 as vehicle_prototype is a struct, not a class. It seems not to affect most compilers, but VS would bitch about this when linking.

ZhilkinSerg pushed a commit that referenced this pull request Mar 16, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 17, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 17, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 18, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 29, 2020
@ZhilkinSerg ZhilkinSerg mentioned this pull request Apr 1, 2020
13 tasks
ZhilkinSerg pushed a commit that referenced this pull request Apr 1, 2020
ZhilkinSerg pushed a commit that referenced this pull request Apr 1, 2020
ZhilkinSerg pushed a commit that referenced this pull request Apr 2, 2020
ZhilkinSerg pushed a commit that referenced this pull request Apr 2, 2020
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` [JSON] Changes (can be) made in JSON Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants