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

Character Items Bugfix #73

Merged
merged 1 commit into from Feb 17, 2021
Merged

Character Items Bugfix #73

merged 1 commit into from Feb 17, 2021

Conversation

preimpression
Copy link
Contributor

@preimpression preimpression commented Feb 10, 2021

  • Consolidates character limit into populateCategoryData as it was duplicated in two different functions that use populateCategoryData
  • Fixes inability to disable category character ownership (use case: accidental enabling)
    --- Thoughts: Should this handle currently character-owned items?

Tbh I am not sure on the usage of the tertiary and the standalone variable? But it was how it was before so I left it and used the same method for character ownership.

- Consolidates  character limit into `populateCategoryData` as it was duplicated in two different functions that use `populateCategoryData`
- Fixes inability to disable category character ownership (use case: accidental enabling)
--- Thoughts: Should this handle currently character-owned items?
@itinerare itinerare added the needs review Pull requests that are pending community review label Feb 10, 2021
@itinerare
Copy link
Collaborator

Thinking on handling currently character-owned items.... I can see a situation potentially happening where things go real weird and it being automatic is practical. But on the other hand I can also see an accidental toggle-off going really sideways with that sort of automation in place.

@preimpression
Copy link
Contributor Author

Perhaps a standalone button + function or even a console command to remove + pass the items back to their players? One that gives plenty of warning like "This many characters have an item in this category. Are you sure? Are you really sure?"

@itinerare
Copy link
Collaborator

Yeah I think if anything a standalone solution would be wisest

@itinerare itinerare merged commit 64b825f into corowne:develop Feb 17, 2021
@itinerare itinerare added reviewed Pull requests that have received community review and are pending merge and removed needs review Pull requests that are pending community review labels Feb 17, 2021
@preimpression preimpression deleted the patch-1 branch February 22, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed Pull requests that have received community review and are pending merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants