Coding Style

[satoon101]

Hhhmmm... I find these ones veeeeeeeeery ugly:

Syntax: Select all

# Python Imports
#   OS
from os.path import dirname
from os.path import join

# Source.Python Imports
...

Why not just leave it to that?

Syntax: Select all

# Python Imports
from os.path import dirname
from os.path import join

# Source.Python Imports
...

Looks easier to read IMRHO... AAAAAAAAAND, secondly, these

Syntax: Select all

#   OS

and stuff really DON'T explain anything...

Again, this is for the API only. Use whatever you wish in your own scripts. I find separating the imports to be much easier to read. Also, OS explains that you are now starting the imports from the "os" module. You might find it ugly, but I find proper separation much more pleasing to the eyes than just lumping everything together.

Satoon

[Tuck]

That will happen if you try to import it in IDLE. Now that you attempted to import it, it should have created a .pyc file for it. Whatever folder you installed the pep8.py file into, there should now be a __pycache__ folder. Go into that folder, and you should see a file with pep8 in the name, with the extension .pyc.

What I typically do is take the .pyc file, and put it in its own directory (I use D:\pep8\). Once you have it in the directory you wish to have it in, open your command console and navigate to that folder. Then, use the following in the console (this is using my pep8 directory as well as the location of my server's addons):

Code: Select all

C:\Windows\system32>d:

D:\>cd pep8

D:\pep8>pep8.pyc d:/servers/csgo/csgo/addons/source-python/_libs

Satoon

i guess u want me to run the setup.py first than, however it wants setuptools but i cant find setuptools for python 3 :/, found http://pypi.python.org/pypi/distribute however even this errors when i execute it..

Traceback (most recent call last):
File "C:\Users\Tuck\Downloads\distribute_setup.py", line 515, in <module>
main(sys.argv[1:])
File "C:\Users\Tuck\Downloads\distribute_setup.py", line 510, in main
tarball = download_setuptools()
File "C:\Users\Tuck\Downloads\distribute_setup.py", line 193, in download_setuptools
log.warn("Downloading %s", url)
File "C:\Python32\lib\distutils\log.py", line 47, in warn
self._log(WARN, msg, args)
File "C:\Python32\lib\distutils\log.py", line 30, in _log
if stream.errors == 'strict':
AttributeError: errors

(idle did not create a pep8.pyc or a __pycache__ folder for some reason)

[BackRaw]

Again, this is for the API only. Use whatever you wish in your own scripts. I find separating the imports to be much easier to read. Also, OS explains that you are now starting the imports from the "os" module. You might find it ugly, but I find proper separation much more pleasing to the eyes than just lumping everything together.

Satoon

I thought the Coding Style applies to all scripts?! lol, okay then... but

Syntax: Select all

from os.path import join

already explains that you're importing from the OS module... seperating them is great - but you only need one blank line!

I was not saying that you just lump everything together... my idea would look like this:

Syntax: Select all

# Python Imports
from configobj import ConfigObj

from os.path import dirname
from os.path import join

from math import atan
from math import pi

# Source.Python Imports
from Source import Engine
from Source import Misc

and yours like that:

Syntax: Select all

# Python Imports
#   Configobj
from configobj import ConfigObj
#   OS
from os.path import dirname
from os.path import join
#   Math
from math import atan
from math import pi

# Source.Python Imports
#    Source
from Source import Engine
from Source import Misc

see the difference? - which one looks cleaner? ;)

[satoon101]

I personally think mine looks cleaner. I also do not like the separation without a comment explaining the separation.

Satoon

[satoon101]

Are you certain? Mine created the __pycache__ folder. You do not need to run the setup.py at all.

Ok, try this. Copy the pep8.py (and only that one file) to your C:\Python32\Lib\site-packages\ folder. Then, in IDLE, use import pep8 and that will definitely create the __pycache__ folder inside the site-packages folder (if it doesn't already exist) and create a pep8.cpython-32.pyc file. If it doesn't, then do note that I am using Python3.3, so maybe that could be the difference. Once that file has been created, change its name to pep8.pyc and move it to its own directory.

Satoon

[Tuck]

uninstalled 3.2 and downloaded python 3.3 however it still wouldnt, create a pyc running it in idle however, importing it created the pyc.

But i've finally got it working thanks a bunch satoon101.

[Mahi]

Lines must be no longer than 80 characters (that includes the built-in \n, making it 79 characters of code)
Satoon

Why is the \n included here, any reasoning at all?

Also, just curious, is there a reason you prefer not to use[PYTHON]from os.path import dirname, join, curdir[/PYTHON]Which is obviously shorter, and pretty much as clean as having three separate lines which all import from os.path anyways?

[L'In20Cible]

The main point here is to keep your code human readable. Easier to follow multiple lines than a single one with no ending (especially when working in more than one files side by side which most of us does).

[satoon101]

For the first, because that is what PEP8 uses as a standard. For the second, just personal preference, partly due to the fact that you could have so many objects to import that it needs to go to another line (due to the 80 character limit). For example:

Syntax: Select all

from os.path import (dirname, join, curdir, basename, exists, isfile, isdir,
    splitdrive, splittext)

That just looks extremely ugly to me. Having them on separate lines looks a whole lot better.

[Doldol]

I dislike PEP8's 80 char limit per line and never follow it, I find that it breaks up the logic of your program just to look nicer.

As for readability, we're in 2015, it's not hard to find an editor that can do dynamic word wrapping, it has the advantages of always wrapping at the correct spot, and in most cases it doesn't increase line nr., doesn't commit the wrap to the file and displays it clearly and nicely indented.

The only disadvantage I see is when people manually word wrap their code negating the advantages I summed up and risking having code show up potentially wrapped twice.

The only case I can see for manually wrapping is when defining data structures (lists, dicts, etc).

[Mahi]

I dislike PEP8's 80 char limit per line and never follow it, I find that it breaks up the logic of your program just to look nicer.

As for readability, we're in 2015, it's not hard to find an editor that can do dynamic word wrapping, it has the advantages of always wrapping at the correct spot, and in most cases it doesn't increase line nr., doesn't commit the wrap to the file and displays it clearly and nicely indented.

The only disadvantage I see is when people manually word wrap their code negating the advantages I summed up and risking having code show up potentially wrapped twice.

The only case I can see for manually wrapping is when defining data structures (lists, dicts, etc).

I highly agree with you. I hope we're allowed to write longer lines into our plugins :P

[satoon101]

I feel I need to reiterate this, but these are guidelines for the plugin itself not for scripters to adhere to for their plugins.

I highly disagree with you both, and there are plenty of ways to properly break your code up to look nice without going over the 80 character limit. And I also feel the need to point this out:
https://www.python.org/dev/peps/pep-0008/#id10

Just because there are editors that handle word wrapping doesn't mean that you should just write 1000 character lines. It is still a pain to read and understand if you don't make those clear breaks yourself. And why would you force people to have to get a text editor with wrapping capabilities in order to view your code? That doesn't seem like good coding practice to me at all. I also absolutely hate seeing \ to continue a line on the next one.

If you notice, the SP repository includes a checker.bat file that runs PEP8, PEP257, pyflakes, and pylint. pylint is a fantastic tool that I think everyone should take advantage of. When using these checkers, they also tell you when you are using improper indents with continued lines. So, for instance:

Syntax: Select all

# This is obviously valid
def testing(args):
    ...


# This is valid
def testing(
        one, two, three, four, five, six, seven):
    ...


# This is valid
def testing(one, two, three,
            four, five, six, seven):
    ...


# This is invalid
def testing(
    one, two, three, four, five, six, seven):
    ...


# This is invalid
def testing(one, two, three,
        four, five, six, seven):
    ...

If you run that code through the pep8 checker, this will be the result:

Code: Select all

C:\Windows\system32>python -m pep8 E:\test.py
E:\test.py:20:5: E125 continuation line with same indent as next logical line
E:\test.py:26:9: E128 continuation line under-indented for visual indent

C:\Windows\system32>

[Doldol]

I feel I need to reiterate this, but these are guidelines for the plugin itself not for scripters to adhere to for their plugins.

I understand that, I guess I really felt like bouncing off my (unpopular) views off of people here.

This is more clear to me, than anything:

Syntax: Select all

def testing(one, two, three, four, five, six, seven):
    ...

I am defining a function, with the name of testing, with these arguments.
My next lines will define what the function does.

I stand by this even if I had 1000 arguments, it still logically belongs there.
And I absolutely agree with your view on \.

But honestly I don't terribly mind it much when browsing to this project's source code, I guess I'm mostly discussing the general usage of this. Apologies on getting off-topic.

[satoon101]

I feel I need to reiterate this, but these are guidelines for the plugin itself not for scripters to adhere to for their plugins.

I understand that, I guess I really felt like bouncing off my (unpopular) views off of people here.

While my statement makes sense to be directed toward you, it was mostly directed towards Mahi's last sentence. There will be more guidelines plugins will need to adhere to than EventScripts has, but I don't think line length is necessarily one of them.

[Mahi]

I feel I need to reiterate this, but these are guidelines for the plugin itself not for scripters to adhere to for their plugins.

Ah yes, that's good news for me.

I highly disagree with you both, and there are plenty of ways to properly break your code up to look nice without going over the 80 character limit.

Sure there are, but just because there are ways, doesn't mean it's always nicer or should be done. Of course some lines look prettier if you break them up, even if they were only 50 characters, and I think everyone should use common sense here. But for example, a list of parameters could easily be wrapped to the next line.

And why would you force people to have to get a text editor with wrapping capabilities in order to view your code? That doesn't seem like good coding practice to me at all.

Not a coding practice, more like development (or evolution? not sure what english word I'm supposed to use here... ), what if your net browsers didn't wrap the paragraphs? Surely if some people ran netbrowsers from 1985, maybe you would have to manually split the paragraphs on multiple lines, but we're already in 2015 and it's obvious for everyone that the text is splitted automatically. Still people often manually split the paragraphs on multiple lines, instead of writing 50 paragraphs one after another. I think this should be the case in programming, it should be an obvious feature of a text editor so nobody would ever have to think "what if he doesn't have wrapping on the next line on his editor?". And surely people would still split some of the code manually.

This is just my opinion, but notice that even windows' notepad supports wrapping if you enable it, I think everyone should start to use it by default (ad if your lines are 80 chars long anyways, there would never be wrapping anyways, so the feature would be useful when reading other people's code and useless when reading or writing your own).

I also absolutely hate seeing \ to continue a line on the next one.

Indeed, \ is one of the most terrible conventions for one to use in python, paranthesis do much better job at splitting a line in many.

[Ayuto]

Never let an editor wrap you code! That's ugliest thing you can do, imo. This will always generate different output depending on the size of your editor window and the type of you editor. Moreover it doesn't split you code logically. Here is an example how it will look like:

Code: Select all

def my_func(param1, param2, param3
,param4, param5):
    # Code...

How do you know the second line wasn't an idention error made by the programmer? It should actually look like this:

Code: Select all

def my_func(param1, param2, param3,
        param4, param5):
    # Code...

[Mahi]

Never let an editor wrap you code! That's ugliest thing you can do, imo. This will always generate different output depending on the size of your editor window and the type of you editor. Moreover it doesn't split you code logically. Here is an example how it will look like:

Code: Select all

def my_func(param1, param2, param3
,param4, param5):
    # Code...

How do you know the second line wasn't an idention error made by the programmer? It should actually look like this:

Code: Select all

def my_func(param1, param2, param3,
        param4, param5):
    # Code...

You have a point there, but the editor should show you that the line is wrapped (usually some sort of arrows in the beginning and the end of the line). Also in your example, the comma between param3 and param4 has been misplaced by the programmer, that's not wrapping's fault, and it looks bad whether you use automatic wrapping or not.

[BackRaw]

I have another question: should scripters use constants for player property strings and that kind of stuff? I came up with 4 possibilities, and I'm not sure what's best to read for both scripters and those who want to understand the code.

#1 - use strings

Syntax: Select all

# myplugin/players.py

PlayerEntity(index).set_property_int("m_iPlayerState", 0)
PlayerEntity(index).set_property_float("m_flFlashMaxAlpha", 0.0)

#2 - use constants in the py file that the constants are used in only

Syntax: Select all

# myplugin/players.py

# Flash property constants
PROPERTY_FLASH_MAXALPHA = "m_flFlashMaxAlpha"
PROPERTY_FLASH_DURATION = "m_flFlashDuration"
PROPERTY_FLASH_AMMO = "localdata.m_iAmmo.012"

# Player property constants
PROPERTY_PLAYER_STATE = "m_iPlayerState"

# ....

PlayerEntity(index).set_property_int(PROPERTY_PLAYER_STATE, 0)
#...

#3 - constants in their own file

Syntax: Select all

# myplugin/constants.py

# Flash property constants
PROPERTY_FLASH_MAXALPHA = "m_flFlashMaxAlpha"
PROPERTY_FLASH_DURATION = "m_flFlashDuration"
PROPERTY_FLASH_AMMO = "localdata.m_iAmmo.012"

# Player property constants
PROPERTY_PLAYER_STATE = "m_iPlayerState"

Syntax: Select all

# myplugin/players.py

from myplugin.constants import PROPERTY_PLAYER_STATE

# use it

#4 (does it work that way? lol)

Syntax: Select all

# myplugin/players/__init__.py

# Flash property constants
PROPERTY_FLASH_MAXALPHA = "m_flFlashMaxAlpha"
PROPERTY_FLASH_DURATION = "m_flFlashDuration"
PROPERTY_FLASH_AMMO = "localdata.m_iAmmo.012"

# Player property constants
PROPERTY_PLAYER_STATE = "m_iPlayerState"

Syntax: Select all

# myplugin/players/entity.py

from myplugin.players import PROPERTY_FLASH_MAXALPHA

# use it

Syntax: Select all

# myplugin/players/somethingelse.py

from myplugin.players import PROPERTY_FLASH_DURATION

# ..

I guess it makes sense to use #3 if you use constants along your script in other files and #4 makes sense if you need them only in the specific package. But if you just need them in one file, then I'm not sure what to say about #2...

I like the way #define works in C++ and I can see the use for it and it's a way cleaner code. However, in Python, my opinion is that strings are cleaner lol. On the other hand, the reason why there are constants can be said about Python, too.

[satoon101]

I would say whatever you choose works best for you. Again, these guidelines are specific to the plugin itself and not for everyone's scripts.

When we first started this plugin, we had thought that many people might post some good additions to the plugin itself. Since we wanted to keep everything fairly uniform, and so that we didn't have to go back through anyone's code, we decided to post this thread for them to reference. Then, if we had any issues with the formatting of the code, we could just post a link to this thread so they could update it to our standards. Unfortunately, very little was posted and what was didn't really fit with our gameplan for the specific areas that were involved. This thread still serves that purpose for anyone who does post code, either here or as a pull request on GitHub.

Back to your post specifically. It is my intention that the next commit I make will include both m_flFlashMaxAlpha and m_flFlashDuration in the entities data.

The flashbang count (m_iAmmo.012) is already available:

Syntax: Select all

count = <PlayerEntity>.get_flashbang_count()

<PlayerEntity>.set_flashbang_count(2)

I am not sure about m_iPlayerState. I will have to look more into that property to see how we want to account for it.

[Ayuto]

I would also like to add that using constants will make your life easier in some situations.
1. If the value of the constant need to be changed, you only need to change one occurence. If you don't use constants you would need to do that everywhere you use this value.
2. Constants also allow you to give values a name. Imagine you use the value "5" somewhere in your code. At the first glance you won't know what the value is representing. If you would create a constant for it (e.g. PLAYER_SPEED) it would be absolutely clear what the value represents.