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.

Sign up for GitHub

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

Jump to bottom

Add More Conditions to runif #320

Open

wilyt1 wants to merge 2 commits into evaera:master from wilyt1:runif-conditions

Collaborator

wilyt1 commented Dec 16, 2023

This pull request expands on the features mentioned in issue #25 by adding more conditions.

Declarations:

I declare that this contribution was created in whole or in part by me.
I declare that I have the right to submit this contribution under the terms of this repository's license and declarations.
I understand and agree that this contribution and a record of it are public, maintained permanently, and may be redistributed under the terms of this repository's license.


          Add More Conditions to runif

0566caa

Krcnos suggested changes

View reviewed changes

Contributor

Krcnos left a comment

Solid addition to the runif command! Just a few feedback changes to make it more intuitive and complete

Cmdr/BuiltInCommands/Utility/runif.lua

+              	end,
+              	greaterThan = function(text, arg)
+              		if tonumber(text) > tonumber(arg) then

Contributor

Krcnos Mar 5, 2024

At the moment you'd have to do runIf greaterThan 1 2 command and it would return true as 2 is greater than 1 but I think tonumber(text) > tonumber(arg) should be switched to tonumber(arg) > tonumber(text) to be a little more intuitive as you're comparing the argument against the text

Cmdr/BuiltInCommands/Utility/runif.lua

+              	end,
+              	lessThan = function(text, arg)
+              		if tonumber(text) < tonumber(arg) then

Contributor

Krcnos Mar 5, 2024

Same as line 39 comment, switch them

Cmdr/BuiltInCommands/Utility/runif.lua

+              	end,
+              	greaterThanOrEqual = function(text, arg)
+              		if tonumber(text) >= tonumber(arg) then

Contributor

Krcnos Mar 5, 2024

Same as line 39 comment, switch them

Cmdr/BuiltInCommands/Utility/runif.lua

+              	end,
+              	lessThanOrEqual = function(text, arg)
+              		if tonumber(text) <= tonumber(arg) then

Contributor

Krcnos Mar 5, 2024

Same as line 39 comment, switch them

Cmdr/BuiltInCommands/Utility/runif.lua Show resolved Hide resolved

autonordev self-assigned this

Collaborator

autonordev commented Apr 26, 2024

@wilyt1 have you considered the comments from krcnos?

Collaborator Author

wilyt1 commented Apr 29, 2024

I've reviewed the feedback and explained why implementing those suggestions wouldn't work right now, mainly because of how the runif command is set up. You can check out the details here: link to the Discord conversation.


          Merge branch 'evaera:master' into runif-conditions

4f32ffe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet