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

NPC work log - for debugging and feedback #37300

Closed
wants to merge 5 commits into from
Closed

NPC work log - for debugging and feedback #37300

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 22, 2020

Summary

SUMMARY: Features "NPC work log - for debugging and feedback"

Purpose of change

Adds a work log that lists NPC behaviour changes and mission changes and activity failures and reasons etc, to aid in debugging and giving feedback to the player.

Describe the solution

I kept promising myself I would add something like this before I added any new features to NPCs, debugging them and working out where they were going wrong was an exercise in frustration, I wanted a proper NPC activity test system too, but I thought something like this would help in that endeavour.

What this does is add a std::deque work log , every change in attitude, mission, activity and certian steps in activities, adds an entry to the log, with timestamp, character id, and a description of what changed, sometimes a tripoint and activity_reason_info too.

If debug mode is on, it lists everything, else, it just lists activity stuff.
Two ways to access it currently - from a basecamp bulletin board ,this will prefilter the log to show just the NPCs that are assigned to work at that camp.

You can also ask an NPC directly to tell you what their work log is, itll prefilter to just that NPC.

[WIP] because its rough, I would like to make it easier to read, prettier, easier to filter from within the window that pops up, perhaps add more categories of event, and ways to access it, so as to extract more relevant information and less filler.

Willing to accept guidance in this, but as it stands currently, its still useful for finding out why Bob failed to remove that vehicle part, or why he suddenly decided his mission was to run away or something, just gives a bit of an insight into their mind.

Describe alternatives you've considered

N/A

Testing

Asked NPC to do various things, assigned them to camp, opened work log, did the smae by talking to them directly.
Did this with debug mode on and off.

Tested save/load cycle to see that serial/deserial worked.

Additional context

Basic UI stuff.

image

src/npc.cpp Outdated Show resolved Hide resolved
src/npc.cpp Outdated Show resolved Hide resolved
src/npc.cpp Outdated Show resolved Hide resolved
src/npc.cpp Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/savegame.cpp Outdated Show resolved Hide resolved
src/savegame.cpp Outdated Show resolved Hide resolved
src/savegame_json.cpp Outdated Show resolved Hide resolved
src/savegame_json.cpp Outdated Show resolved Hide resolved
src/savegame_json.cpp Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/savegame_json.cpp Outdated Show resolved Hide resolved
src/savegame_json.cpp Outdated Show resolved Hide resolved
src/activity_handlers.h Outdated Show resolved Hide resolved
@ZhilkinSerg ZhilkinSerg added 0.E Feature Freeze <Enhancement / Feature> New features, or enhancements on existing [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Info / User Interface Game - player communication, menus, etc. NPC / Factions NPCs, AI, Speech, Factions, Ownership labels Jan 25, 2020
@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/basecamp-what-do-i-need-to-make-butchering-job-actually-work/22565/3

@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/how-does-npc-butchery-work/22654/11

@ZhilkinSerg ZhilkinSerg force-pushed the dev branch 5 times, most recently from 924f105 to 39e00e3 Compare March 18, 2020 07:00
@ZhilkinSerg ZhilkinSerg force-pushed the dev branch 3 times, most recently from ad63e77 to 8e68539 Compare April 1, 2020 12:18
@kevingranade kevingranade force-pushed the dev branch 4 times, most recently from 621a68e to b7106d0 Compare April 2, 2020 07:42
@ZhilkinSerg ZhilkinSerg force-pushed the dev branch 3 times, most recently from 0f30a43 to d432807 Compare April 2, 2020 12:55
@ZhilkinSerg ZhilkinSerg changed the base branch from dev to master April 2, 2020 14:31
davidpwbrown added 5 commits April 4, 2020 15:15
first attempt at making an NPC work log

serializing and expanding reasons sent to log

clear folded list on display

from review : shortern inserts

Update src/npc.cpp

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

Update src/npc.cpp

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

Update src/npc.h

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

Update src/npc.cpp

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

remove unneeded lines

test refactor to use enum strings

move enum to string definitions to cpp from header

Update src/savegame_json.cpp

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

Update src/savegame_json.cpp

Co-Authored-By: Jianxiang Wang (王健翔) <qrox@sina.com>

fix up enum to string conversoin and default constructor

added work log result for no candidates spaces to move components / cant see

Update activity_item_handling.cpp

shortern is_npc() checks

commit to trigger rebuild
@ghost ghost changed the title [WIP]NPC work log - for debugging and feedback NPC work log - for debugging and feedback Apr 4, 2020
@ghost
Copy link
Author

ghost commented Apr 4, 2020

Rebased, cleaned up a bit, tested, working.

Still lots of refinements and additions that could be made to this, but the framework is there and the diff is getting big, as a first pass I consider this ready.

@Qrox
Copy link
Contributor

Qrox commented Apr 5, 2020

Could you consider also migrating the UI code to use ui_adaptor/ui_manager?

@ghost
Copy link
Author

ghost commented Apr 5, 2020

Could you consider also migrating the UI code to use ui_adaptor/ui_manager?

Not familiar with that, and UI is my least favourite thing to do lol, but I'll take a look

if( con_idx ) {
json.member( "con_idx", con_idx->id() );
} else {
json.member( "con_idx", -1 );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
json.member( "con_idx", -1 );
json.null_member( "con_idx" );

For consistency with how cata::optional is written.

JsonObject jo = jsin.get_object();
reason = jo.get_enum_value<do_activity_reason>( "reason" );
jo.read( "can_do", can_do );
if( jo.has_int( "index " ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if( jo.has_int( "index " ) ) {
if( jo.has_null( "con_idx" ) ) {

@ZhilkinSerg
Copy link
Contributor

Feel free to reopen when you come back.

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` <Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants