Pull request for sample_npc.h, sample_npc.c, genstruct.c, and default_room #639
Conversation
navila-luna
added
enhancement
|
Hey, for some reason this includes a bunch of changes to WDL files- specifically, |
navila-luna
changed the title
There was a problem hiding this comment.
Looks pretty good! I'm worried that you might be experiencing some sort of compile problem that is stopping you from including files properly. Also, there has been quite a few changes to gen_structs.h. I know you make some changes to the structs as well, but a few of the definitions dont exist anymore. Besides for that, the npc files look good! Just make sure to not have tab characters and use 4 spaces for each new line!
There was a problem hiding this comment.
When you submit or review a PR, please check the diff output for all the changes that were made- many of my comments could have been avoided
About 1500 of the 1900 lines changed in this PR were spaces being converted to tabs (which is not allowed in the chiventure style guide). It's much harder to focus on both the design and technical details of the code when seeing there other changes, which means that more in-depth passes will need to be made
| HASH_FIND_STR(tmp, "olive", item); | ||
| cr_assert_str_eq(item->short_desc, "a black olive"); | ||
| cr_assert_str_eq(item->long_desc, "A singular, unrefrigerated black olive"); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Let's remove changes made to other files if they aren't a part of this PR
There was a problem hiding this comment.
I wasn't sure what because the merge conflict wasn't very clear (specific lines, some even just two lines) if they were old or new so i just replace it with what bryan had pr for which had all tests passing and working.
…cs/chiventure into openworld/samples-npcs Considering the suggestions made by Dawson.
|
Ok so far, I have done the majority of the suggestion(spaces, spelling, comments, and tests). I have yet to change the function I npc_lookup. I'm still trying to figure out what is meant by adding the npc name instead of a pointer. I'm thinking that strncpy could work. |
|
A number of my comments from the previous pass at the PR have not been addressed yet, and most of them relate to using tabs as indentation. Additionally, it looks like there are more places where this is the case, or other cases where whitespace and styling have been modified. Again, please check the diff output for all the changes that were made during this pull request- this is under the "files" tab for this PR. As it stands right now, the amount of changes to the code due to whitespace is keeping me from performing an in-depth review of both design and technical implementation of the code, which is a requirement for me to approve this PR. There's about 1800 lines of changes in this PR, but I'd estimate about 1200 of them are just whitespace issues |
It appears so that whenever I'm editing, since rn i'm editing through git hub that it puts it tabbing even when i did spaces.
I find it a bit weird that there were tab issues here as i only modified the room spec struct fixed a merge conflict
Pull request for the inclusion of sample_npc.h and sample_npc.c. In addition, modifications to the genstruct.h and default_room.c file More information about the application of the sample_npc module can be found in issues #562 and #563