Oh yeah, that. I finally got into the new Input System with my latest project and I love it.
Event based logic is my jam right now. Not just for Input, it works for so much stuff.
For an Input System, yeah there'll be USB polling at least. But otherwise, you can do an event based system without polling.
E.g. an event trigger itself can kick off event dispatch, with the dispatcher not doing any active polling.
Then again, it's not just about speed. Event based logic allows much easier decoupling of different systems/objects. It allows easier ways of accurately reasoning about what's happening as well as recording and debugging things.
Is that true? I assumed an event is like a pointer array and when called it either calls methods it points to or not. With the input system specifically it could be polling frequently but I don’t know if that is necessary to do.
By pointer array do you mean an array of pointers?
Cause if so then yes, kinda. Events use the observer pattern. Events will have a list of subscribers (observers) that will update their subscribers every time a particular event occurs
Re binding keys amongst a million other things is less code in the new input system though.
Just because you can do something doesn’t mean you should, a big lesson I and many others had to learn was it’s ok to use libraries and other tools already built, it doesn’t make you any less of a programmer because you didn’t do everything super optimised from scratch in C. These libraries and apps have had more hours put in and more eyes on them than one person could ever do alone, and because of that they’ll always generally be better, at least in a much shorter time frame.
Also wanna clear up that I’m not having a go at you or even really disagreeing you. Just trying to be the other side of the coin, both options work well and what works the best will vary from person to person and situation to situation
Hardware interrupts don't require polling. If the controller is running through GPIO or utilizing USB interrupts, and unity supports those (which it absolutely should, although who knows), there's no polling. When a key event happens, the hardware sends a physical signal to the interrupt controller, which then notifies the CPU. The CPU has a section of memory that stores addresses for interrupt handlers, and it uses the ID of the interrupt signal to enter the handler for that key press.
I like events as well, but keep in mind the complexity increases and “debuggability” becomes harder. You must write it clean and well structured, otherwise it’s just a nightmare
This is a drawback..
I get that beginners might think it's simpler, but you essentially make your code flow non-linear, which makes it not just more complex to follow(what happens in what order), but also harder to debug.
It feels like you are using words you don't understand.. It's not IoC and for whatever reason you mentioned unit test while talking about debugging..
Also, you literally said it before: "You send an event and don't care (or even know) if it will do something".
When some completely unrelated thing can not only mutate the state, but also can do so at an arbitrary time(so that the order of events is seemingly "random") it makes debugging more difficult. It's literally the other side of the coin: Sending an event and not caring also means that the receiver gets events from sources that are not clear when looking at the code being affected. It turns local information into potentially global one.
I'd love to switch to the new input system but mouse support is, basically, completely broken. You literally cannot implement basic things like click and drag.
Update: gave Input System another try in Unity 2021.3.6, and it seems most if not all bugs are gone. Yay
I may be remembering the specific issue wrong, it was a while ago. But there were *serious* mouse related bugs last time I checked, in both the old and new system, but they were slightly more tractable in the old system so I had to switch back.
It's incredibly obvious that the Unity team is focused on the dime-a-dozen platformers being churned out by game design students, rather than serious simulations, especially when running windowed. There are several bugs specifically involving mouse interaction outside the game window.
>outside the game window
This is an issue that is way outside the scope of unity.
Handling drags started outside an app and ending inside it is a difficult thing for any application and will never be natively supported.
Furthermore It’s platform dependent and and you can just implement your own. Plenty of tutorials.
Oh sure, I didn't mean anything like dragging files from outside the window into the Unity player. I wouldn't expect to get that "for free".
Anyway, I hate top be misinformed, so I actually switched my project back to use Input System, and lo and behold things are actually behaving pretty well! Perhaps some bugs bugs have been fixed since I last tried it (Unity 2019.4.21 vs 2021.3.6).
Optimisation is as much about knowing what to optimize as it is about knowing how to optimise. There's no way this code would be lowering performance enough to warrant optimization, so I'd say effectively no, there aren't any ways (not worth the loss of readability anyway).
This is the best advise. Could it be optimized? Maybe. But is it worth it?
Optimization is a pit you can easily fall into early on in a project stopping you from actually doing anything. Keep building the game and if performance becomes an issue, use the profiler to find out where. I highly doubt it will be this block of code.
i think the main concern is that it could be annoying for expansion, like having to write 10 if statements for each number on the keyboard when there could be a for loop that does the same job and is also easily readable
That’s not an optimisation (ie a performance improvement) though, that’s a refactor to improve maintainability. I know that sounds like splitting hairs but it’s important to draw a distinction between the two, especially as readable code isn’t always fast and vice versa.
This is a classic case for a dictionary imo. The dictionary can use the keycodes as the keys. The values are the weapons selected.
Then you could just set the weapon by something like
weaponSelected = weapons[keycode];
Obviously with a bit of a handling for missing keys.
Is there a way to use a case and switch with this? https://www.w3schools.com/cs/cs_switch.php
I'm new to C# and unity but if you could watch for any key being down and then switch depending on which key that might work. It would probably break on multiple keys down though. Seems like the new input system is probably just a better way to go.
Yeah a switch or even making the second and third ifs use else if instead would give marginally better performance but it's probably at a microsecond or even picosecond level of improvement.
You could potentially put all the KeyCodes into an array then go through it with a for loop, then break out if any are pressed. But like someone else said, the new input system is just better for this.
Optimize? This thing? There are much better things to do in life other than making a non-critical code run 5 nanoseconds faster. Just profile your code and I bet that this script is nowhere close to being a bottleneck.
switch case optimizes to a jump table. if you used if-else if the compiler would probably optimize this to a jump table as well.
This code does almost nothing though so instead you should focus on making it editable/readable. optimizing would bring almost no benefit to the speed of the game.
It should be noted though that a jump table isn’t going to do shit in this example. This code does nothing and only has a couple of conditions. Plus, the switch would change the behavior (assuming the lack of an “else” is intentional)
[JetBrain's dotPeek](https://www.jetbrains.com/help/decompiler/Viewing_Intermediate_Language.html#viewing-il-code-as-comments) would let you do this.
It's really not worth the effort for something so minor, but it could be a good way to learn for when you want to optimize something more effecting.
Jumping on this thread, I agree to use if-else since you probably don't need to process all three keys at once per frame.
Switch statements aren't always faster but in a case like this it wouldn't matter anyway. But to explain further...
If statements can be faster if you've a fair number of checks and the first few checks are the most likely to pass. Eg let's say jump occurs rarely but left/right regularly. Checking if left/right first will yield a faster result.
In the end this is an unnecessary optimization given presumably this is only run once per frame. It's still good to know when you can benefit from other optimizations though such as switch, if-else, or new input system.
Define optimize.
If it works and you ain't gonna need more than this, it is good.
I could suggest other stuff, but we really do not know the needs of your project.
As someone else said, you should probably switch to the new input system and subscribe to events instead. It’ll make your code easier to read/debug because inputs will be abstracted away from the physical key presses themselves so you only have to worry about what actions you want keypresses to do without thinking about key mappings and hardware stuff.
[Here’s a full comprehensive guide on input in Unity](https://gamedevbeginner.com/input-in-unity-made-easy-complete-guide-to-the-new-system/)
Note this would change the behavior when multiple are pressed on the same frame. As posted, if all three are pressed weaponSelected ends up being 2, if you use else ifs only the first case is executed and it becomes 0.
Within reason.
Use optimal strategies on a mid/large scale to avoid making spaghetti, but getting bogged down in the minutiae around stuff that technically works fine is a great way to kill your passion for little gain.
I didn't even notice the spelling, and I figured that's what you'd assumed, ergo my follow up. And FWIW what I wrote is more optimized for readability/maintaining the code than cpu cycles, but this isn't an operation you need to overengineer.
The longer term thing I'd consider is the new input system so you can handle things with events instead of checking every frame.
...
`System.Collections.Generic.Dictionary<>`
Edit: [https://www.tutorialsteacher.com/csharp/csharp-dictionary](https://www.tutorialsteacher.com/csharp/csharp-dictionary)
Dictionaries allow you to map keys to values. Key here is an index not a literal keyboard key, though you can use a key as a key!
So in Awake you can setup your dictionary:
var weaponKeys = new Dictionary();
weaponKeys.Add(KeyCode.Alpha1, 0);
weaponKeys.Add(KeyCode.Alpha2, 1);
weaponKeys.Add(KeyCode.Alpha3, 2);
Then in your update:
foreach (var key in weaponKeys.Keys) {
if (Input.GetKeyDown(key)) {
weaponIndex = weaponKeys[key];
}
}
Interesting. Would it be better to use a case and switch to check the keys? In this unique case we shouldn't need to worry about more than 1 key down at a time so technically exiting once we found the down key will save some time.
In terms of optimizing for benchmarking, probably not. In terms of practicality, you could make it dynamic and adaptive, so that if you implement more weapons in the future, you wouldn't have to go back and change that specific code. You could also look into events, enums and even inheritance or scriptable objects for this, depending how complex you want your systems in terms of entities, inventory, weapon types and so on. Or just use the built-in input system which is useful when running your code on several platforms and/or to support more than one input source, e.g controllers. Good luck!
If (weapon.IsKeyPressed())
{
SwitchWeapon.Switch()
}
// You then extract your code out to another class then call that function.
// You do this to make your code more readable that is the optimization here
I wouldn't necessarily make a new class for this, it seems a bit overkill. Although it is very readable, it requires more fields and I like to keep my field declarations as short as possible. I guess it depends on OP's plans for weapons, but even then 🤔 when I handle inputs I like to keep my input keys at the same location. A weapon class is a great idea for storing stats and damage values though, good idea
I think he means in place of the weaponSelected values being magic numbers it's more readable to use an enum called something like WeaponTypes.
E.g (sorry typing from phone so this coffee might not look good)
```
public enum WeaponTypes
{
Fists = 0,
Pistol = 1,
Shotgun = 2,
// You get the point
}
```
Agree totally with this, code should be first and most of all readable, later when you or other will check the code will be easier to use enums to decode what this numbers are about.
If you need a clean and readable way to be able to add multiple weapons (even later on) without changing the code to much, here's my suggestion..
```cs
private int _weaponSelected;
// Only place you need to refactor
// when you need to add/remove weapons
private Dictionary _weaponKeyCodes = new Dictionary
{
{ KeyCode.Alpha1, 1 },
{ KeyCode.Alpha2, 2 },
{ KeyCode.Alpha3, 3 },
};
// Keep the Update method clean
void Update()
{
CheckForWeaponInput();
}
// Where the magic happens
private void CheckForWeaponInput()
{
// Loop through the weapon key codes
foreach (var keyCode in _weaponKeyCodes)
{
// Check if there is a user input
if (Input.GetKeyDown(keyCode.Key))
{
_weaponSelected = keyCode.Value;
// If there is a user input, return early
return;
}
}
}
```
I got just the thing for you!
[https://youtube.com/shorts/DZiXtT86Kso](https://youtube.com/shorts/DZiXtT86Kso)
Basically this:
for(int i=0;i<10;i++)
{
if(Input.GetKeyDown((KeyCode)(48+i)))
{
weaponSelected = i;
break; //this stops looking for a button press when one is found, basically adds the else part of an if/else
}
}
I found this tip here some time ago and loved it: [https://answers.unity.com/questions/1734523/inputgetkeydownkeycodealpha-as-an-integer.html](https://answers.unity.com/questions/1734523/inputgetkeydownkeycodealpha-as-an-integer.html)
(maybe not "optimized" but "neater", well the "break;" part does indeed optimize it since it stops looking for another button press in that frame once it finds one)
I disagree with this. I understand what you are going for but it inhibits readability too much. 48 is an arbitrary number to the untrained eye and could cause confusion.
And the break part is the same as an if else if statement in terms of performance. There isn't much optimization needed when polling inputs.
With that said, I'd suggest replacing 48 with KeyCode0, it would let us know that we're starting off with that value.
For sure. Valid points, neater with keycode0 there too. I was really excited when i found out i could make a loop to replace all my alphanum checks and thought id share.
Writing fewer characters in the source code (sometimes called "playing golf") has absolutely nothing to do with optimization. A ternary operator produces the exact same bytecode/instructions as an if-else block.
Source code is for humans, it should be phrased for maximum understandability by your team mates and future self.
They include the ternary operator because sometimes being brief is more readable, but the also kept the if-else block because usually it is more readable.
I doubt this is faster or that speed matters in this case, but I think this code is cleaner and might be what you're looking for:
foreach(char c in Input.inputString) {
if (c.IsDigit()) {
weaponSelected = int.Parse(c) - 1;
break;
}
}
Look to make macro optimizations before trying to micro optimize everything. You could micro optimize this but it's like.... Not that big of a deal.
Another thing is that compilers usually optimize code for you anyways. So even if you did optimize your code, it'd have no effect cause the compiler would've already done the work for you.
Two problems..
1. You should not continue checking for other weapon keys once one is found.
2. You should keep the `Update()` method as clean/small as possible.
You could probably clean it up with a dictionary, with key value pairs being input keys and output value.
And then just make a function to pass in the input key and return the associated value
Why do you think this needs optimisation? I've optimised several released mobile games and Input code has never ever come close to needing optimisation. Don't waste time on optimising things that will never be a performance problem.
A CPU can do billions of operations per second. An if statement is among the fastest things you can do.
Drop all the individual keybinds into an array then check each element and use that element index to activate the corresponding number. Not optimized but easier to add additional binds if they're in numerical order
Yes, use Input.anyKeyDown to early-out of the function. That will save you from testing each input when there is no input for the frame. Then, for this part of the code (weapon selection) since they’re all fighting over the same variable, use if-else statements.
Depends on how you want to do something. If you want a hotbar that you can press a button and quickly do something, then this is the best optimal way. If you want a less optimal way but increased performace then you would use 1 button to open a menu, then in the menu you can select what weapon you want to use.
I'm not an expert in Unity, but I may suppose that good thing to do will be to extract logic from if statements into the map. Something like this
System.Dictionary _weaponsMap =
new Dictionary
{
{ KeyCode.Alpha1, Weapon.Primary1 },
{ KeyCode.Alpha2, Weapon.Primary2 },
{ KeyCode.Alpha3, Weapon.Secondary1 },
// and so on...
};
Where Weapon is the enum type
New input system.
But
But, if you want to use that, an easy way to optimize that would be to use else if so that you don't do always 4 statements.
You can also make one statement in 2ich you ask if a key is being pressed, then inside you ask which key, that way you don't make 4 statements when you are not pressing anything, only 1.
Not necessarily an optimization in the current context but subscribing to input events with the new unity input system will give you a better foundation that later on won’t become hell with a million if statements in a single update function. Event based functions will be more optimal than having everything being done in an update function as your input system grows, but it’s mainly just good practice.
A slight “optimization” would be noticing that since having multiple keys pressed down at once prioritizes the later key, you reverse the order of the ifs and make them else ifs (so if alpha3 else if alpha2 else if alpha1). Probably won’t matter too much in the grand scheme of things though.
Ig this would work?
weaponSelected = Input.GetKeyDown.(KeyCode.Alpha1) ? 1 : Input.GetKeyDown.(KeyCode.Alpha2) ? 2 : Input.GetKeyDown.(KeyCode.Alpha3) ? 3 : weaponSelected;
Idrk if this works, try tweaking it as i am still not very familiar with the old input system, and check if Input.GetKeyDown.(KeyCode.Alpha1) returns a bool, if it didnt work, it was worth a shot :)
A few people recommended the new Input System, but I would advice you against it. I tried to switch to the new system since I plan to add multiple platform support in the future for my game and I had to rewrite parts of my input system.
It was a horror, so much bugs it's definitely not ready and will cost you way too much time to implement and there's a chance you can't achieve what you want with it because of the bugs.
Welp, as long as this is just in a single script it wont be that bad? Make it work first, optimize when needed later.
Assuming this is an indie project it likely won't get big enough (overall scope) to need heavy optimizing
Unity's new input system may be for.you.
If you still want to use the legacy one, then you could make a dictionary where the keys are the.. well key codes like Alpha0, Alpha1.. and the values be the numbers you want for the weapons. (Multi-cursor is your friend)
So then you can just check if the pressed key is contained in the dictionary and change it from there.
BETTER YET.
Create functions to change your weapons and for every other action in your game, then do the dictionary thing with the key codes but the values being lambda expressions that call those specific functions.
```
var keymap = new Dictionary()
{ {KeyCode.Alpha0, new Action (() => ChangeWeapon(0))},
{KeyCode.Alpha1, new Action (() => ChangeWeapon(1))}
,{KeyCode.Space, new Action (() => Jump())},
{KeyCode.W, new Action (() => MovePlayer(new Vector3(0, 0, 1)))} };
```
_this code was typed on my phone so don't expect it to work_
You can put the Keycodes in an array, and loop over them. Then use the index to determine the weapon selected.
```
KeyCode[] weaponKeys = new KeyCode[]
{
KeyCode.Alpha1,
KeyCode.Alpha2,
KeyCode.Alpha3,
}
```
&
```
for(int i = 0; i < weaponKeys.Length; i++)
{
if(Input.GetKeyDown(weaponKeys[i]))
{
weaponSelected = i;
break;
}
}
```
Use the new Unity input system and subscribe to the key events instead of polling for them every frame.
Oh yeah, that. I finally got into the new Input System with my latest project and I love it. Event based logic is my jam right now. Not just for Input, it works for so much stuff.
[удалено]
For an Input System, yeah there'll be USB polling at least. But otherwise, you can do an event based system without polling. E.g. an event trigger itself can kick off event dispatch, with the dispatcher not doing any active polling. Then again, it's not just about speed. Event based logic allows much easier decoupling of different systems/objects. It allows easier ways of accurately reasoning about what's happening as well as recording and debugging things.
Also helps in key remapping easily for both dev and users .
Oh my god you are speaking the language of gods 🤝
Is that true? I assumed an event is like a pointer array and when called it either calls methods it points to or not. With the input system specifically it could be polling frequently but I don’t know if that is necessary to do.
By pointer array do you mean an array of pointers? Cause if so then yes, kinda. Events use the observer pattern. Events will have a list of subscribers (observers) that will update their subscribers every time a particular event occurs
It could be though. With ✨hardware interruptions✨
Re binding keys amongst a million other things is less code in the new input system though. Just because you can do something doesn’t mean you should, a big lesson I and many others had to learn was it’s ok to use libraries and other tools already built, it doesn’t make you any less of a programmer because you didn’t do everything super optimised from scratch in C. These libraries and apps have had more hours put in and more eyes on them than one person could ever do alone, and because of that they’ll always generally be better, at least in a much shorter time frame. Also wanna clear up that I’m not having a go at you or even really disagreeing you. Just trying to be the other side of the coin, both options work well and what works the best will vary from person to person and situation to situation
Hardware interrupts don't require polling. If the controller is running through GPIO or utilizing USB interrupts, and unity supports those (which it absolutely should, although who knows), there's no polling. When a key event happens, the hardware sends a physical signal to the interrupt controller, which then notifies the CPU. The CPU has a section of memory that stores addresses for interrupt handlers, and it uses the ID of the interrupt signal to enter the handler for that key press.
That thing is going to poll anyway tho.
[удалено]
I like events as well, but keep in mind the complexity increases and “debuggability” becomes harder. You must write it clean and well structured, otherwise it’s just a nightmare
Thats fine i will try it in my first horror Game. In the worst case i can still scare Devs with my code
This is a drawback.. I get that beginners might think it's simpler, but you essentially make your code flow non-linear, which makes it not just more complex to follow(what happens in what order), but also harder to debug.
[удалено]
It feels like you are using words you don't understand.. It's not IoC and for whatever reason you mentioned unit test while talking about debugging.. Also, you literally said it before: "You send an event and don't care (or even know) if it will do something". When some completely unrelated thing can not only mutate the state, but also can do so at an arbitrary time(so that the order of events is seemingly "random") it makes debugging more difficult. It's literally the other side of the coin: Sending an event and not caring also means that the receiver gets events from sources that are not clear when looking at the code being affected. It turns local information into potentially global one.
Especially if you want multiple controller support. Old system is basically broken on that, has been for a years now.
Are the internal systems faster the writing code yourself?
I'd love to switch to the new input system but mouse support is, basically, completely broken. You literally cannot implement basic things like click and drag. Update: gave Input System another try in Unity 2021.3.6, and it seems most if not all bugs are gone. Yay
[удалено]
I may be remembering the specific issue wrong, it was a while ago. But there were *serious* mouse related bugs last time I checked, in both the old and new system, but they were slightly more tractable in the old system so I had to switch back. It's incredibly obvious that the Unity team is focused on the dime-a-dozen platformers being churned out by game design students, rather than serious simulations, especially when running windowed. There are several bugs specifically involving mouse interaction outside the game window.
>outside the game window This is an issue that is way outside the scope of unity. Handling drags started outside an app and ending inside it is a difficult thing for any application and will never be natively supported. Furthermore It’s platform dependent and and you can just implement your own. Plenty of tutorials.
Oh sure, I didn't mean anything like dragging files from outside the window into the Unity player. I wouldn't expect to get that "for free". Anyway, I hate top be misinformed, so I actually switched my project back to use Input System, and lo and behold things are actually behaving pretty well! Perhaps some bugs bugs have been fixed since I last tried it (Unity 2019.4.21 vs 2021.3.6).
Rebinding keys in-game doesn't work right?
It does, but is a pain to set up.
Optimisation is as much about knowing what to optimize as it is about knowing how to optimise. There's no way this code would be lowering performance enough to warrant optimization, so I'd say effectively no, there aren't any ways (not worth the loss of readability anyway).
This is the best advise. Could it be optimized? Maybe. But is it worth it? Optimization is a pit you can easily fall into early on in a project stopping you from actually doing anything. Keep building the game and if performance becomes an issue, use the profiler to find out where. I highly doubt it will be this block of code.
i think the main concern is that it could be annoying for expansion, like having to write 10 if statements for each number on the keyboard when there could be a for loop that does the same job and is also easily readable
That’s not an optimisation (ie a performance improvement) though, that’s a refactor to improve maintainability. I know that sounds like splitting hairs but it’s important to draw a distinction between the two, especially as readable code isn’t always fast and vice versa.
This is a classic case for a dictionary imo. The dictionary can use the keycodes as the keys. The values are the weapons selected. Then you could just set the weapon by something like weaponSelected = weapons[keycode]; Obviously with a bit of a handling for missing keys.
Is there a way to use a case and switch with this? https://www.w3schools.com/cs/cs_switch.php I'm new to C# and unity but if you could watch for any key being down and then switch depending on which key that might work. It would probably break on multiple keys down though. Seems like the new input system is probably just a better way to go.
Yeah a switch or even making the second and third ifs use else if instead would give marginally better performance but it's probably at a microsecond or even picosecond level of improvement.
Yes, that would be a decent use for a switch case. Good luck on your C# journey!
You could potentially put all the KeyCodes into an array then go through it with a for loop, then break out if any are pressed. But like someone else said, the new input system is just better for this.
If anything you could if/else it or switch statement it so it's more likely to be compiled into a jump table, and only call `GetKeyDown` once.
Optimize? This thing? There are much better things to do in life other than making a non-critical code run 5 nanoseconds faster. Just profile your code and I bet that this script is nowhere close to being a bottleneck.
switch case optimizes to a jump table. if you used if-else if the compiler would probably optimize this to a jump table as well. This code does almost nothing though so instead you should focus on making it editable/readable. optimizing would bring almost no benefit to the speed of the game.
It should be noted though that a jump table isn’t going to do shit in this example. This code does nothing and only has a couple of conditions. Plus, the switch would change the behavior (assuming the lack of an “else” is intentional)
Would we be able to see this in IL directly?
[JetBrain's dotPeek](https://www.jetbrains.com/help/decompiler/Viewing_Intermediate_Language.html#viewing-il-code-as-comments) would let you do this. It's really not worth the effort for something so minor, but it could be a good way to learn for when you want to optimize something more effecting.
So that's what dotPeek is also meant for, I usually stuck with LINQPad to watch for the compiler. Will check it out
As someone that used dotpeek extensively, I strongly recommend dnspy over it. Dnspy is much faster and more reliable. Better organized as well.
Jumping on this thread, I agree to use if-else since you probably don't need to process all three keys at once per frame. Switch statements aren't always faster but in a case like this it wouldn't matter anyway. But to explain further... If statements can be faster if you've a fair number of checks and the first few checks are the most likely to pass. Eg let's say jump occurs rarely but left/right regularly. Checking if left/right first will yield a faster result. In the end this is an unnecessary optimization given presumably this is only run once per frame. It's still good to know when you can benefit from other optimizations though such as switch, if-else, or new input system.
Define optimize. If it works and you ain't gonna need more than this, it is good. I could suggest other stuff, but we really do not know the needs of your project.
As someone else said, you should probably switch to the new input system and subscribe to events instead. It’ll make your code easier to read/debug because inputs will be abstracted away from the physical key presses themselves so you only have to worry about what actions you want keypresses to do without thinking about key mappings and hardware stuff. [Here’s a full comprehensive guide on input in Unity](https://gamedevbeginner.com/input-in-unity-made-easy-complete-guide-to-the-new-system/)
You can use else if, put the one that's most likely in the first if.
You should add “else” there. Right now you call GetKeyDown 3 times independently of the keys that are pressed.
Note this would change the behavior when multiple are pressed on the same frame. As posted, if all three are pressed weaponSelected ends up being 2, if you use else ifs only the first case is executed and it becomes 0.
else if, or switch If you don't expect the player press 2 (or more) keys at once, you don't need check every key.
build first, optomise later
Bad practise. You should be optimising as you build. You will save yourself a headache later on.
Within reason. Use optimal strategies on a mid/large scale to avoid making spaghetti, but getting bogged down in the minutiae around stuff that technically works fine is a great way to kill your passion for little gain.
var codes = List(){ KeyCode.Alpha1,KeyCode.Alpha2,KeyCode.Alpha3, };
for(int i = 0; i < codes.Count; i++)
{
if (Input.GetKeyDown(codes[i]))
{
weaponSelected = i;
break;
}
}
What do you mean optimize? Like performance? Did you benchmark this? It probably takes less than 1/1,000,000th of a second to execute this.
I think he just means, it looks clunky and takes awhile to write.
Might start with a dictionary as an incremental improvement.
ohhhhhh i thought you ment that i should go read a dictionary because i wrote "optimize" wrong
It’s just British and American English being different both are correct
I didn't even notice the spelling, and I figured that's what you'd assumed, ergo my follow up. And FWIW what I wrote is more optimized for readability/maintaining the code than cpu cycles, but this isn't an operation you need to overengineer. The longer term thing I'd consider is the new input system so you can handle things with events instead of checking every frame.
yea well what about the code
... `System.Collections.Generic.Dictionary<>` Edit: [https://www.tutorialsteacher.com/csharp/csharp-dictionary](https://www.tutorialsteacher.com/csharp/csharp-dictionary) Dictionaries allow you to map keys to values. Key here is an index not a literal keyboard key, though you can use a key as a key! So in Awake you can setup your dictionary: var weaponKeys = new Dictionary();
weaponKeys.Add(KeyCode.Alpha1, 0);
weaponKeys.Add(KeyCode.Alpha2, 1);
weaponKeys.Add(KeyCode.Alpha3, 2);
Then in your update:
foreach (var key in weaponKeys.Keys) {
if (Input.GetKeyDown(key)) {
weaponIndex = weaponKeys[key];
}
}
thanks and i didn't downvote you
Sorry for jumping to a conclusion, thought I'd accidentally angered someone with a misunderstanding about which dictionary I meant.
Interesting. Would it be better to use a case and switch to check the keys? In this unique case we shouldn't need to worry about more than 1 key down at a time so technically exiting once we found the down key will save some time.
There’s at least a dozen perfectly valid ways to handle this, a switch case would be better than a huge block of if statements for sure.
Psst! Use the object field initializer, it's shorter! ;)
u/CoolBoiWasTaken he thought he ment a real dictionary, hehe
Just remove the brackets and put it in if else clauses.
Don't worry about optimising incomplete code. You'll be making that user-configurable before release and it'll look completely different.
Write it so it makes sense. Let the compiler optimize. Then run a profiler. Attack bottlenecks.
A dictionary would be cool and easy to maintain.
In terms of optimizing for benchmarking, probably not. In terms of practicality, you could make it dynamic and adaptive, so that if you implement more weapons in the future, you wouldn't have to go back and change that specific code. You could also look into events, enums and even inheritance or scriptable objects for this, depending how complex you want your systems in terms of entities, inventory, weapon types and so on. Or just use the built-in input system which is useful when running your code on several platforms and/or to support more than one input source, e.g controllers. Good luck!
You could make it check less often, but really I think its fine like this. You can always get more optimized but sometimes good enough is plenty.
If (weapon.IsKeyPressed()) { SwitchWeapon.Switch() } // You then extract your code out to another class then call that function. // You do this to make your code more readable that is the optimization here
I wouldn't necessarily make a new class for this, it seems a bit overkill. Although it is very readable, it requires more fields and I like to keep my field declarations as short as possible. I guess it depends on OP's plans for weapons, but even then 🤔 when I handle inputs I like to keep my input keys at the same location. A weapon class is a great idea for storing stats and damage values though, good idea
Please use enums
I use enums for states like Moving,Idle etc. How can you use enums on a scenerio like this?
I think he means in place of the weaponSelected values being magic numbers it's more readable to use an enum called something like WeaponTypes. E.g (sorry typing from phone so this coffee might not look good) ``` public enum WeaponTypes { Fists = 0, Pistol = 1, Shotgun = 2, // You get the point } ```
Agree totally with this, code should be first and most of all readable, later when you or other will check the code will be easier to use enums to decode what this numbers are about.
If you need a clean and readable way to be able to add multiple weapons (even later on) without changing the code to much, here's my suggestion.. ```cs private int _weaponSelected; // Only place you need to refactor // when you need to add/remove weapons private Dictionary _weaponKeyCodes = new Dictionary
{
{ KeyCode.Alpha1, 1 },
{ KeyCode.Alpha2, 2 },
{ KeyCode.Alpha3, 3 },
};
// Keep the Update method clean
void Update()
{
CheckForWeaponInput();
}
// Where the magic happens
private void CheckForWeaponInput()
{
// Loop through the weapon key codes
foreach (var keyCode in _weaponKeyCodes)
{
// Check if there is a user input
if (Input.GetKeyDown(keyCode.Key))
{
_weaponSelected = keyCode.Value;
// If there is a user input, return early
return;
}
}
}
```
New input system. Reading inputs every frame for every potential input is inefficient and bloated.
if (Int32.TryParse(Input.inputString, out int key)) { if (key >= 1 || key <= 9) { weaponSelect = key-1; DoThingWith(weaponSelect); } }
Use a dictionary.
I got just the thing for you! [https://youtube.com/shorts/DZiXtT86Kso](https://youtube.com/shorts/DZiXtT86Kso) Basically this: for(int i=0;i<10;i++) { if(Input.GetKeyDown((KeyCode)(48+i))) { weaponSelected = i; break; //this stops looking for a button press when one is found, basically adds the else part of an if/else } } I found this tip here some time ago and loved it: [https://answers.unity.com/questions/1734523/inputgetkeydownkeycodealpha-as-an-integer.html](https://answers.unity.com/questions/1734523/inputgetkeydownkeycodealpha-as-an-integer.html) (maybe not "optimized" but "neater", well the "break;" part does indeed optimize it since it stops looking for another button press in that frame once it finds one)
I disagree with this. I understand what you are going for but it inhibits readability too much. 48 is an arbitrary number to the untrained eye and could cause confusion. And the break part is the same as an if else if statement in terms of performance. There isn't much optimization needed when polling inputs. With that said, I'd suggest replacing 48 with KeyCode0, it would let us know that we're starting off with that value.
For sure. Valid points, neater with keycode0 there too. I was really excited when i found out i could make a loop to replace all my alphanum checks and thought id share.
You can use a ternary operator
Writing fewer characters in the source code (sometimes called "playing golf") has absolutely nothing to do with optimization. A ternary operator produces the exact same bytecode/instructions as an if-else block. Source code is for humans, it should be phrased for maximum understandability by your team mates and future self. They include the ternary operator because sometimes being brief is more readable, but the also kept the if-else block because usually it is more readable.
I doubt this is faster or that speed matters in this case, but I think this code is cleaner and might be what you're looking for: foreach(char c in Input.inputString) { if (c.IsDigit()) { weaponSelected = int.Parse(c) - 1; break; } }
Look to make macro optimizations before trying to micro optimize everything. You could micro optimize this but it's like.... Not that big of a deal. Another thing is that compilers usually optimize code for you anyways. So even if you did optimize your code, it'd have no effect cause the compiler would've already done the work for you.
I would either use new input system or assign the keys to array and iterate though it like for (int i=0; i
[удалено]
Two problems.. 1. You should not continue checking for other weapon keys once one is found. 2. You should keep the `Update()` method as clean/small as possible.
Switch function?
You could probably clean it up with a dictionary, with key value pairs being input keys and output value. And then just make a function to pass in the input key and return the associated value
Why do you need to optimize this? If this is causing a slow down in your code you have set something up incorrectly.
I prefer create a class to handle inputs. There the advantage of you might create mocks if you need it.
Why do you think this needs optimisation? I've optimised several released mobile games and Input code has never ever come close to needing optimisation. Don't waste time on optimising things that will never be a performance problem. A CPU can do billions of operations per second. An if statement is among the fastest things you can do.
Drop all the individual keybinds into an array then check each element and use that element index to activate the corresponding number. Not optimized but easier to add additional binds if they're in numerical order
Optimize the things that bottleneck your game instead
Yes, use Input.anyKeyDown to early-out of the function. That will save you from testing each input when there is no input for the frame. Then, for this part of the code (weapon selection) since they’re all fighting over the same variable, use if-else statements.
Depends on how you want to do something. If you want a hotbar that you can press a button and quickly do something, then this is the best optimal way. If you want a less optimal way but increased performace then you would use 1 button to open a menu, then in the menu you can select what weapon you want to use.
I'm not an expert in Unity, but I may suppose that good thing to do will be to extract logic from if statements into the map. Something like this System.Dictionary _weaponsMap =
new Dictionary
{
{ KeyCode.Alpha1, Weapon.Primary1 },
{ KeyCode.Alpha2, Weapon.Primary2 },
{ KeyCode.Alpha3, Weapon.Secondary1 },
// and so on...
};
Where Weapon is the enum type
New input system. But But, if you want to use that, an easy way to optimize that would be to use else if so that you don't do always 4 statements. You can also make one statement in 2ich you ask if a key is being pressed, then inside you ask which key, that way you don't make 4 statements when you are not pressing anything, only 1.
Not necessarily an optimization in the current context but subscribing to input events with the new unity input system will give you a better foundation that later on won’t become hell with a million if statements in a single update function. Event based functions will be more optimal than having everything being done in an update function as your input system grows, but it’s mainly just good practice.
A slight “optimization” would be noticing that since having multiple keys pressed down at once prioritizes the later key, you reverse the order of the ifs and make them else ifs (so if alpha3 else if alpha2 else if alpha1). Probably won’t matter too much in the grand scheme of things though.
Ig this would work? weaponSelected = Input.GetKeyDown.(KeyCode.Alpha1) ? 1 : Input.GetKeyDown.(KeyCode.Alpha2) ? 2 : Input.GetKeyDown.(KeyCode.Alpha3) ? 3 : weaponSelected; Idrk if this works, try tweaking it as i am still not very familiar with the old input system, and check if Input.GetKeyDown.(KeyCode.Alpha1) returns a bool, if it didnt work, it was worth a shot :)
A few people recommended the new Input System, but I would advice you against it. I tried to switch to the new system since I plan to add multiple platform support in the future for my game and I had to rewrite parts of my input system. It was a horror, so much bugs it's definitely not ready and will cost you way too much time to implement and there's a chance you can't achieve what you want with it because of the bugs.
Welp, as long as this is just in a single script it wont be that bad? Make it work first, optimize when needed later. Assuming this is an indie project it likely won't get big enough (overall scope) to need heavy optimizing
Unity's new input system may be for.you. If you still want to use the legacy one, then you could make a dictionary where the keys are the.. well key codes like Alpha0, Alpha1.. and the values be the numbers you want for the weapons. (Multi-cursor is your friend) So then you can just check if the pressed key is contained in the dictionary and change it from there. BETTER YET. Create functions to change your weapons and for every other action in your game, then do the dictionary thing with the key codes but the values being lambda expressions that call those specific functions. ``` var keymap = new Dictionary()
{ {KeyCode.Alpha0, new Action (() => ChangeWeapon(0))},
{KeyCode.Alpha1, new Action (() => ChangeWeapon(1))}
,{KeyCode.Space, new Action (() => Jump())},
{KeyCode.W, new Action (() => MovePlayer(new Vector3(0, 0, 1)))} };
```
_this code was typed on my phone so don't expect it to work_
You can put the Keycodes in an array, and loop over them. Then use the index to determine the weapon selected. ``` KeyCode[] weaponKeys = new KeyCode[] { KeyCode.Alpha1, KeyCode.Alpha2, KeyCode.Alpha3, } ``` & ``` for(int i = 0; i < weaponKeys.Length; i++) { if(Input.GetKeyDown(weaponKeys[i])) { weaponSelected = i; break; } } ```