Post Processing on Professions #39019
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Post processing on profession selection and generation on game start.
damien
reviewed
Mar 30, 2020
Comment on lines
+431
to
+452
| /* Purpose: Post processing on profession selection and generation on start. | ||
| Professions are newly generated each time on selection, even | ||
| if the profession has already been selected previously. The profession | ||
| is even generated again after selection, and the game is started. | ||
| The purpose of this loop is to provide default numbers for item charges | ||
| and ammo. This extends to food as well, however due to the nature of | ||
| the way these professions are generated, food items one see's on the profession | ||
| screen will be different in-game, but still these charge numbers will be | ||
| dictated by the default of said items. | ||
|
|
||
| Graphical Issue: Currently, worn items can fluctuate between 0 and their default value | ||
| upon entering the game this issue will resolve itself and the correct | ||
| default value will be given to the player. This issue occurs | ||
| on holding the enter key to make rapid selection of a profession. | ||
|
|
||
| TODO: Currently the way magazines are implemented they do not contain a default value. | ||
| Professions contain a max charge value found in the JSON file however, this | ||
| does not help in choosing a default. Below is a compromise that takes the | ||
| half of the magazines capacity as a default. This was chosen because most | ||
| defaults are half of the items max value. | ||
| -- Ideally, the default value of a magazine would be defined in the profession JSON file. -- | ||
| */ |
There was a problem hiding this comment.
👏 👏 👏
Always happy to seed thorough documentation. Well done on this feature, and good on you for writing stellar docs! 🎉
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Post processing on profession selection and generation on game start.
Summary
SUMMARY: Bugfixes "Changes starting charge/ammo values on items and is now a correct reflection of what will be given to the player in game."
Purpose of change
Fixes #38675 - Adds post processing to profession generation, allowing for correct representation of charge/ammo values for items that will be given to the player at the start of the game. Fix up for debate on final values
Describe the solution
Added post processing to profession::items() stopping randomization that was happening during selection of the profession and again at the start of the game. This would result in a player receiving starting items/charges/ammo that was not indicative of what they chose on profession selection.
This was accomplished through a couple iterators grabbing values for said items and setting them to their default values, it does this for every item on the character.
Describe alternatives you've considered
Currently the way magazines are implemented they do not contain a default value.
Professions contain a max charge value found in the JSON file however, this does not help in choosing a default. This fix is a compromise that takes the half of the magazines capacity as a default. This was chosen because most defaults are half of the items max value.
Ideally, the default value of a magazine would be defined in the profession JSON file.
Testing
Testing was done in-game on all professions that had items that could be randomized such as charge's/ammo/food. This change only modifies values of said items, this code will be executed during selection of the profession and during the moment the player selects confirm and the game is started.
Further programmatic testing and unit tests was not done due to my inexperience with such a large project. I felt the above testing was extensive enough and since the change is minor could be safely integrated into the project without said unit tests. However, during this time I will continue attempting to develop unit testing skills, and consult with the boys down at the discord.
Additional context
Personally I believe profession generation code should be run once during selection for any unique chosen profession. As it stands, if the user holds the enter key, the chosen profession is generated as fast as the program will let them. This information should be stored somewhere and accessed later on start of the game. Maybe a follow up to the optimization of the character generation could be further looked at.
This is my first contribution to a big project, go easy on me :) hope to work with you guys more in the future, thanks for all the help in learning the code base. This fix could be greatly improved and modified so don't hesitate to speak up, I'd like to know your thoughts.