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

CharacterController has allowGod option #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

CharacterController has allowGod option #31

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 31, 2014

No description provided.

@ianballantyne
Copy link

Seems like a fair addition. Please can you make the following changes:

  1. Setting the params.allowGod = false, appears to still allow god mode
  2. Can you change the '{' bracket of the setGod function to be on the next line to match the Turbulenz style guidelines.
    Thanks for contributing!

@ghost
Copy link
Author

ghost commented Apr 3, 2014

I don't get the 1.

Ian Ballantyne wrote:

Seems like a fair addition. Please can you make the following changes:

  1. Setting the params.allowGod = false, appears to still allow god mode
  2. Can you change the '{' bracket of the setGod function to be on the
    next line to match the Turbulenz style guidelines.
    Thanks for contributing!


Reply to this email directly or view it on GitHub
#31 (comment).

@ianballantyne
Copy link

c.allowGod = params.allowGod || c.allowGod;

in this case if params.allowGod is false (or undefined), then allowGod will still be true. It should probably be:

if (params.allowGod !== undefined)
{
c.allowGod = params.allowGod;
}

@ghost
Copy link
Author

ghost commented Apr 3, 2014

Oh... yes.

Ian Ballantyne wrote:

c.allowGod = params.allowGod || c.allowGod;

in this case if params.allowGod is false (or undefined), then allowGod
will still be true. It should probably be:

if (params.allowGod !== undefined)
{
c.allowGod = params.allowGod;
}


Reply to this email directly or view it on GitHub
#31 (comment).

@ghost
Copy link
Author

ghost commented Apr 3, 2014

fixed

@ghost ghost closed this Apr 3, 2014
@ghost ghost reopened this Apr 3, 2014
@ghost
Copy link
Author

ghost commented Apr 7, 2014

@ianballantyne I fixed your two points.

@ianballantyne
Copy link

@Herby Thanks. We'll take a look shortly.

@davidgaleano
Copy link
Contributor

I would suggest to change the option to be "disableGod", it makes the logic a bit simpler.

@ghost
Copy link
Author

ghost commented Apr 15, 2014

done

David Galeano wrote:

I would suggest to change the option to be "disableGod", it makes the
logic a bit simpler.


Reply to this email directly or view it on GitHub
#31 (comment).

@davidgaleano
Copy link
Contributor

We usually write line 149 like this:

    c.disableGod = params.disableGod || false;

@ghost
Copy link
Author

ghost commented Apr 15, 2014

Whatever you see fit.

David Galeano wrote:

We usually write line 149 like this:

| c.disableGod = params.disableGod || false;
|


Reply to this email directly or view it on GitHub
#31 (comment).

@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
@turbulenz turbulenz deleted a comment Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants