-
Notifications
You must be signed in to change notification settings - Fork 23
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
fixed and refactored set_show_item -> display_item #1036
base: develop
Are you sure you want to change the base?
Conversation
fix resistance tests
fix null-pointer crash
some mild refactoring, but no real changes.
https://bugs.eressea.de/view.php?id=2980 Ausserdem: Flammenschwerter vor Laenschwertern bevorzugen.
Co-authored-by: Enno Rehling <[email protected]>
Co-authored-by: Enno Rehling <[email protected]>
Sehr schön, sogar mit Tests!
* narrow down failing test' * on failure, print the filename
let install script use cmake --install
this resolves issue eressea#1026
Co-authored-by: Enno Rehling <[email protected]>
* upgrade to python3 * get rid of python2 compatibility --------- Co-authored-by: Enno Rehling <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im großen und ganzen in Ordnung, denke ich. Eine stylistische Anmerkung hatte ich.
int | ||
report_plaintext(const char *filename, report_context * ctx, | ||
const char *bom) | ||
{ | ||
int age, flag = 0; | ||
char ch; | ||
char potions_shown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warum ist das hier kein int? Der natürliche Typ für Zahlen ist int, char für Zeichen. Wir müssen hier keine Bytes auf dem Stack sparen, die sind wahrscheinlich sowieso 32-bit aligned. Ein char zu benutzen führt nur dazu, dass in der Signatur von report_item ein char * steht, was normal einen Buffer für einen String signalisiert, es macht also das Lesen des Codes notwendig, um zu verstehen, was dort als Parameter erwartet wird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Den Kommentar mit char potions_shown
bitte kurz ändern, dann akzeptiere ich den PR.
The server tried to display starting items like ROI as potions and failed. Moved the display_item/potion/race stuff to reports.c