Potion class, and how it is encoded#86
Potion class, and how it is encoded#86zacharyLYH wants to merge 13 commits intomakeopensource:v2.0from
Conversation
emilkovacev
left a comment
emilkovacev
left a comment
There was a problem hiding this comment.
Great PR! I made a lot of suggestions, feel free to reply to any of them if you have follow up questions.
One additional thing – please remove the LICENSE and .DS_Store files
README.md
Outdated
| 1) Know the attributes of your object (duh) | ||
|
|
||
| 2) Go to **builtin.py** -> **Obj** and find **Obj** class. This is where you initialize the object. **NOTE:** You don't assume the player is going to use an object. This is merely initialization. Tip: Initialize the logic of your object here. | ||
|
|
||
| 3) The hard part is done - go nuts and build your own class definition. | ||
|
|
||
| 4) Do not forget to go to **crpg.py** -> **generate()** iff you are: | ||
| - creating a new object type. | ||
| - go to the first for loop and look for the first if statement. | ||
| - remember to include your new object type in n_types |
There was a problem hiding this comment.
I think we should steer away from instructions on modifying the code itself. Instead, we can comment above and within the functions we write so that devs can understand exactly what each function does!
zacharyLYH
changed the title
emilkovacev
left a comment
emilkovacev
left a comment
There was a problem hiding this comment.
Excellent improvements! It's almost ready to go, once these things are changes I'm ready to merge :)
builtin.py
Outdated
| if "-health-collect" in title: | ||
| self.amount_health = int(title[19:21]) | ||
| self.potionType = "Health potion" |
There was a problem hiding this comment.
Could we have classes that inherit the potion class for Health, Speed, etc, instead of this?
There was a problem hiding this comment.
Got it. Check out the recent push. I made it as generic as possible, but notice that it is impossible to encode "this is a health potion worth 25 hp" without actually calling it out and subsequently using conditionals to decipher it. However, the implementation of Potion is certainly cleaner now.
There was a problem hiding this comment.
Could you add is_health_potion as an argument? Another approach would be to have an action method with different implementations for each class
crpg.py
Outdated
| n, n_type, *args = re.split(r"\s*\|\s*", node) | ||
|
|
||
| if n_type == "fight" or n_type == "run": | ||
| if n_type == "fight" or n_type == "run" or n_type[:2] == "po": |
There was a problem hiding this comment.
You should be able to specify n_type == "potion" here. n_type stands for node type, and is the second item in the .dl file
README.md
Outdated
| 1) Know the attributes of your potion | ||
|
|
||
| 2) Go to **builtin.py** -> **Potion** and find **Potion** class. This is where you initialize the potion. **NOTE:** You don't assume the player is going to use an object. This is merely initialization. Tip: Initialize the logic of your object here. | ||
|
|
||
| 3) Do not forget to go to **crpg.py** -> **generate()** iff you are: | ||
| - creating a new potion type. | ||
| - go to the first for loop and look for the first if statement. | ||
| - remember to include your new object type in n_types |
There was a problem hiding this comment.
I still think this documentation is a bit out of place here.
…ay all items in the bag as a count of each type of object
…sociated with the potion
2 participants
Proposed changes
All changes and expected Object implementation procedure are outlined in the README. A working example of how to use the currency and potion objects in the .dl file is demonstrated in the .dl file. Note that the logic in the current demonstration of the proposed changes only show the usability of the classes; in reality you'd want to sever the connection to a money object after its been visited but I didn't do that, although a simple fix would be to just change the arrow type.
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. This is simply a reminder of what we are going to look for before merging your code.Further comments
Its not the most elegant way to encode potions, but due to the nature of having many types of potions and the way we currently have the .dl file set up, our options are limited. Also, I forgot to revert the changes I made to .dl, do not forget to add it back after merging.