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

Added Perk, Trait and Profession API #56

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Conversation

aiteron
Copy link
Collaborator

@aiteron aiteron commented Nov 23, 2021

No description provided.

Copy link
Collaborator

@Shurutsue Shurutsue left a comment

Choose a reason for hiding this comment

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

One thing which I'd say is the biggest issue in the changes done:

https://github.com/Konijima/PZ-Community-API/pull/56/files#diff-e1819cf570d6ce02c23cc2c2f4fc2a82b25b3ca7d2166f68dab0c789be5f9589R17-R19
Instead of Shared at the end it's Client?
Is that for use in the Client only - or does that affect the server as well?
If the server requires usage of it as well, it should be shared, otherwise should/could be shoved directly
into the Client folder i reckon.

Aside of that, from a quick glance, there's also:

The Class.__index = Class lines outside the function which sets the metatable should not be needed in these two files:
https://github.com/Konijima/PZ-Community-API/pull/56/files#diff-5fe3c0c113313cab9be3126f3c0f3586137360bc7d6160f5c3dade8f5698b09fR3
https://github.com/Konijima/PZ-Community-API/pull/56/files#diff-0802e12d1e1696eb3df47cd5a0c3b24f94bd13107485b13ae7f839bde2ee950eR3

Additionally missing a new line at the end of various files.

@aiteron
Copy link
Collaborator Author

aiteron commented Nov 25, 2021

https://github.com/Konijima/PZ-Community-API/pull/56/files#diff-e1819cf570d6ce02c23cc2c2f4fc2a82b25b3ca7d2166f68dab0c789be5f9589R17-R19 Instead of Shared at the end it's Client? Is that for use in the Client only - or does that affect the server as well? If the server requires usage of it as well, it should be shared, otherwise should/could be shoved directly into the Client folder i reckon.

This must be shared, because server must know about new traits, perks and professions

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

Successfully merging this pull request may close these issues.

3 participants