Implemented critical hits and weapon specific crit chance system #120
Conversation
I figured out, it would be nice to see the final blow damage, especially when dealing a critical hit. I could potentially use the same variable for all the other methods in Enemy.Java, getting damageDealt from Weapon.java. It would clean up the code a bit, but I don't want to go through the extra hassle, without anyone really demanding it, so if you think it would be a cool enhancement, let me know!
I know you originally wanted to have a 0.01% chance and 10x normal damage, but I think that 1 crit in 10 000 normal hits makes it more of an easter egg than a feature, so I've made it a 1% chance. I've further added some variability to the damage multiplier. This of course is not my project, so if you have any objections, I will change it back to what you have intended, or make an additional if statement with your parameters.
Implemented a new weapon property "critChanceMultiplier" and used it to modify the critical hit logic. I also mentioned this new criteria in "viewAbout()".
| @@ -24,6 +24,7 @@ | |||
| private int coinDropMax; | |||
| private int damageMin; | |||
| private int damageMax; | |||
| int damageDealt; | |||
There was a problem hiding this comment.
Make this a private var, and if you need to access it from another class create a getter for it :)
There was a problem hiding this comment.
Alright, I did not even know something like this existed until now :D I have to say that it is much more clever. Still, I posted a question in the commit resolving this issue. I would greatly appreciate if you answered it.
There was a problem hiding this comment.
Also why can't I just make it a public variable and reference it anywhere? (This is probably common sense for you, but I do not really understand why not)
There was a problem hiding this comment.
When getting the value, why do I have to type "Weapon.get().getDamageDealt()" instead of just "Weapon.getDamageDealt()", when the variable is not directly tied to the weapon?
Great question! This has to do with how Object-Oriented Programming works. Classes need to be initialized/created before you can use them. Think of them as templates. As I'm sure you're aware, you can have multiple instances of Weapon, each with its own values. The exception is static variables/methods, which is tied to the class itself, instead of the instance of the class. Since getDamageDealt() is not static, that means its tied to the instance and you can not access it directly (even though it changes with each round, it technically is still a property of Weapon).
Weapon.get() is a static method for getting the currently "held" weapon. When calling this, you are actually getting the instance of Weapon, and not Weapon itself
Let me know if this is still confusing, I can write a better example if needed!
Also why can't I just make it a public variable and reference it anywhere?
There's nothing "illegal" about doing it this way, it works just fine, a lot of projects will do it this way just because its easier. Though, it is common (and good) practice to not allow direct access to variables. There are plenty of benefits, including:
- Security. It can help prevent accidental changes to the variable (you might only want to allow reading the variable, not changing it)
- You can add validation if needed (using a setter to make sure code isn't trying to set it to a value it shouldn't be)
here's a more technical answer
Note that most of these things usually aren't a problem in smaller projects like this, but become much more important in large projects with dozens of people sharing the same codebase; however, it's always smart to plan ahead and follow good coding styles!
Again, let me know if that still doesn't make sense and I can get better examples for you
There was a problem hiding this comment.
Wow, I did not expect so constructive and comprehensible questions.
I think that I understand for now. I will feel your explanations in practice when programming today and check back here if I need something re-explained.
Thank you so much for the answers, they have really helped me a lot. I am the type of person who does not like doing something he does not understand, so you have essentially made my life a bit easier.
There was a problem hiding this comment.
Thank you so much for the answers, they have really helped me a lot.
No problem! I was in your situation a few years ago, so I definitely understand what it's like; I'm more than happy to help out where I can!
Feel free to reach out whenever, even with questions about your own projects (either code questions, or high-level concepts), or if you just want some code reviews on something! My email is available on my Github profile :)
Definitely feel free to go ahead and implement this! |
When getting the value, why do I have to type "Weapon.get().getDamageDealt()" instead of just "Weapon.getDamageDealt()", when the variable is not directly tied to the weapon?
Again, if you do not like the variable values, let me know.
I do not plan on adding anything else regarding this topic. From what I see, I can only close the pull request and not merge it, so if I am doing something wrong, let me know or feel free to merge the PR yourself. |
Ah didn't realize, alright. Will merge :) Thanks! |
Hey everyone,
this PR should fix the issue #25. I am aware that others tried to implement the feature before me, but since their PR's are open for a few months now, so I have decided to take matter into my own hands.
Since I am fairly new to Java, I am sure that my code will be a bit chaotic and could certainly be executed better. If you happen to find some bugs or some obvious ways to improve the code, be sure to let me know.
I am aware that the crit system is quite overpowered. The best way to solve this issue - in my opinion - is implementing a weapon specific minimum and maximum crit damage multiplier (similar to the crit chance system I've implemented). The reason I did not implement this feature yet, is that it would add more attributes to all the weapons making the code a bit more chaotic. If you do not mind that, I will be more than happy to add it.
Best regards
RindT