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

fixed and refactored set_show_item -> display_item #1036

Draft
wants to merge 28 commits into
base: develop
Choose a base branch
from

Conversation

stm2
Copy link
Member

@stm2 stm2 commented Sep 30, 2023

The server tried to display starting items like ROI as potions and failed. Moved the display_item/potion/race stuff to reports.c

ennorehling and others added 28 commits August 27, 2023 20:38
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.
* narrow down failing test'

* on failure, print the filename
let install script use cmake --install
* upgrade to python3

* get rid of python2 compatibility

---------

Co-authored-by: Enno Rehling <[email protected]>
Copy link
Member

@ennorehling ennorehling left a 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;
Copy link
Member

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.

@ennorehling ennorehling marked this pull request as draft October 9, 2023 07:26
Copy link
Member

@ennorehling ennorehling left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants