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

Fire mode cycling + Coding convensions #2

Closed
wants to merge 5 commits into from

Conversation

InNoHurryToCode
Copy link

Hi!

Used your mod and really like it!
When you mentioned that gamepads would be supported if enough people wanted it, I immediately went to work.

A couple of changes have been made to make it compliant with C#'s Coding Conventions, and to follow general programming idioms (single responsebility).

In addition, I changed the way fire modes are selected in preparation work for gamepad support.
Now users cycle through the fire modes using number key 1.
This way you only have to modify the check for if the fire mode cycle button has been pressed.

I'm uncertain if they use Unity's new input manager, but if they don't getting gamepad support will be challenging:
gamepad layout old input manager

One idea could be to convert the mod by creating new skills instead for the additional fire modes.
If the ability to change fire mode during the run needs to be present, you could use skill_swap to swap the fire skill.

At last, I sadly won't be able to reply promptly.
Feel free to only take the bits you like!

@Vl4dimyr
Copy link
Owner

Hey, I'm glad to hear you like the mod 🙂

And thanks for your work, I hope I can find time soon.

About the changes you made, I would like to keep 1, 2 and 3 and have maybe 4 or some other key for cycling through the modes. I prefer mouse and keyboard, so I have enough keys which can be used for the modes 😋 But having an additional key for cycling through would be nice, and as you already mentioned the logic is needed for the controller support anyway. So I think I will use parts of your changes for this 👍

But let's talk about var I didn't like it in JavaScript and I don't like it in C# (I usually don't work with C#) but why would I use var instead of the correct type? When I read code someone else wrote I hate seeing var since just from a statement like var x = SomeClass.SomeStaticMethod(); you cannot tell the type of x ... Maybe you want to say something about this 😋 For the rest of your changes I'll try to use as much as possible (thanks again for your work).

@Vl4dimyr Vl4dimyr added the enhancement New feature or request label Sep 5, 2020
@Vl4dimyr Vl4dimyr linked an issue Sep 5, 2020 that may be closed by this pull request
@Vl4dimyr
Copy link
Owner

Vl4dimyr commented Sep 5, 2020

Thanks for your work I added controller support!

@Vl4dimyr Vl4dimyr closed this Sep 5, 2020
@InNoHurryToCode
Copy link
Author

InNoHurryToCode commented Sep 7, 2020

Sorry for the late reply, and thank you for looking into my merrge request.
That's amazing news! Can't wait to play with it! 😊

Glad you could cherry-pick some things from it!

Regarding var, the only real reason for me to use it in there is to keep compliance with the Code Guidelines for C#. It imposes no runtime cost like dynamic. If you hover over the type, VS2017 and newer will show the type of var variable in case the type wasn't clear enough 👍.

I do get your displeasure for the var type however, I used to not like it myself either. Only recently discovered it's use when working with reflection. For example in SPTarkov we have to work against variables that changes frequently in type. var makes working against these variables a bless hahaha.

Once again, thanks for your hard work and reply!

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.

Controller Support
2 participants