public, private, protected
) completely differently to the extent that you can see who wrote the code by looking at it.Sooo, what are our guidelines?
My personal guidelines are about this:
* if it's supposed to be called from the outside, make it public
* no specifier otherwise
Also comments, - do we want capitalization and proper punctuation or not?
I always prefer proper English in any situation, especially the use of smileys, ... and !!! in code is just horrible.
>public is the default
How is it more default than
private
? They all have no effect :D>I always prefer proper English in any situation, especially the use of smileys, ... and !!! in code is just horrible.
Ok, nothing at all against that. But we should all do that and possibly add it to our style guidelines?
I think we can rid of protected somehow. Private makes still sense to me.
protected doesn't really make sense because engine callbacks are part of the public interface. There's no reason for a script to not call them.
The only exception might be timer/effect callbacks. But since the engine can also call private, they can as well be private.
We have a horrible amount of regression bugs in scripts. For example, this commit fixes a bug caused by an #appendto accessing an internal variable that was renamed. If internal variables and functions were marked as private, this would not have happened.
>We have a horrible amount of regression bugs in scripts. For example, this commit fixes a bug caused by an #appendto accessing an internal variable that was renamed. If internal variables and functions were marked as private, this would not have happened.
Please be realistic - it would either have happened exactly the same OR the original author would first have removed the
private
specifier... :)At least that is how we have handled such things in C4Script for the last 10 years or so...
PS: also,
public
would mark functions as part of the interface just as well as a missing private
? Or do I misunderstand you here?
> Please be realistic - it would either have happened exactly the same OR the original author would first have removed the private specifier... :)
I think the author would have introduced getters/setters (GetAiming()/SetAiming()) and the person who refactored the script would have seen these functions and adjusted them accordingly.
> If internal variables and functions were marked as private, this would not have happened.
Wouldn't it? If it was still possible (technically), these kind of FYI markups are most likely to be ignored. If we'd again introduce some mechanism for 'private' (like 'do not access via #appendto'), you are still stuck with the same problem: How do I, as the person writing the javelin, determine whether fAiming is an internal variable or part of an interface?
If private was just a markup (that's what it's now, right?) , you'd still have to rely on to things that were ignored in your example: The person accessing the fAiming variable should have thought about writing get/set functions into the javelin; making fAiming private would be a reminder to do that (but could also be ignored what is likely to happen). Otherwise, i.e. fAiming was not private, the person changing it to aiming should have made a complete search for all occurences of fAiming and announce publicly a script-breaking change. There is no reminder to do that and even if, could have been ignored again.
Because without any indication, a scripter can't know which functions are allowed to be used and which are not. So even people who want to do it properly are screwed.
private
is better than public
(as Clonkonaut and I suggested) :I
> Yeah, but this is not really an argument why private is better than public (as Clonkonaut and I suggested) :I
Also, what I mentioned to begin with. I am always very sure when writing a function that I want it to be called from somewhere else. I am never really sure when to forbid calling it from somewhere else. Out of caution you might end up with very few private functions/variables. Or you specifically say that everything that's not definitely intended to be called from elsewhere (which I would mark with public) should be private. Then using public saves some time.
Doesn't really protect you from regression though. It's still very possible for that to happen, only then you can blame someone.
For the default I don't care, except that existing scripts may assume that the default is public, because that is how the engine used to behave.
> except that existing scripts may assume that the default is public, because that is how the engine used to behave.
We should say then, from now on the default is private. I tried to explain my struggle to really say when something's private. People writing #appendto should therefore always expect stuff to change when it's not specifically set to public.
private properties have lowercase names and public interface properties are uppercase.
Currently, most properties follow this convention. Most local variables are lowercase, except for stuff that was moved from the DefCore like "Name", "Description" or "Collectible". They are all part of an interface that shouldn't change.
External objects that store stuff in other objects should also use lowercase. E.g. in adventures you may do stuff like GetCursor().has_visited_smith = true; which is technically public but not part of an interface.
Using properties instead of functions (like SetName()/GetName()) is rare, but may sometimes be done for performance reasons or if overloading should be simplified (e.g.: CreateObject(Firestone).ExplodeSize = 100) or simply when they're used as symbolic constants.
F.e.
Mushroom->Place(..., {size = 100, allow_tunnels = true});
instead of Mushroom->Place(..., {Size = 100, AllowTunnels = true});
At least for me, this looks more intuitive in lower case, especially since I mentally connect CamelCase stuff in scripts with functions.
(This is not a veto against your proposal, which I think is sensible!)
----------------
Methods that are part of a public interface and intended to be called from other objects or scripts, should be prefixed with the
public
keyword. As a part of the public interface of an object public
methods should be documented with comments accordingly.Don't use an access specifier for other methods.
private
and protected
are deprecated.----------------
That would also help us when we potentially want to generate an automatic documentation some day - it would just take all public methods and assume other people are interested in them.
I think the aesthetical aspects of leaving out an access specifier are highly subjective :) Maybe it looks cleaner to you when grouping all the
public
functions at the top of the script so they are not mixed with the others? That'd be a good idea anyway, I guess.I will add something about
global
functions!
global
is already documented, since it's the only specifier that has actual functional meaning for the engine.
The meaning of leaving out an access modifier (which would be private as of your decision) will be "hidden" somewhere in the wiki, in a part of the wiki that is for contributors to OC, not general users of C4Script.
The code however should be understandable for anyone reading it. So if a method should only be called from local context (=is a private method) then this is an important information the reader will want to know without searching for the "meaning" of this in our code guidelines.
Sorry to bring this up again even though you tried to bring this discussion to a result but from this perspective it begs the question where it makes sense at all to NOT supply an access modifier for interface documentation's sake. The only use case I can come up with are quick'n'dirty scripts where the author simply doesn't care about where the methods are called from or some callback functions which aren't really part of the documented interface but still callable from the outside.
In both cases leaving out the modifier would be like 'public', the same as in most other (script) languages. (See my previous point)
> In both cases leaving out the modifier would be like 'public', the same as in most other (script) languages. (See my previous point)
Since the engine ignores all specifiers anyway, there's no need to discuss what leaving it out "means". But if the engine is ever changed to recognize the "private" keyword again, then functions without the keyword will have to be public anyway for compatibility reasons.
Now what about "protected"?
Now, some actually useful "private" functionality would be the ability to have a private func Foo in object Bar, and a global func Foo, where CreateObject(Bar)->Foo() calls the global func, while functions in Bar's script doing Foo() call the private one. That would actually help with resiliency against changes.
Whether this is strictly enforced, just a warning or even done through a convention (stuff like "all private function begin with an underscore"), doesn't matter much. But at least there should be some way to say "don't access this directly" so good scripters can write sane scripts.
> But at least there should be some way to say "don't access this directly" so good scripters can write sane scripts.
Uh, no. That has actually never worked.
When I am writing a scenario or an expansion pack and for some functionality MUST access the internal of one of the original objects, I will always do that. Don't forget that only the core team has the ability to "just quickly write a setter/getter" instead of accessing a local variable directly.
Everyone else either has to use the internals or give up a feature...
And saying "here is a patch guys, I want to release my pack next week and it really needs this getter/setter" is not exactly viable either... Most people will always take features over upward compatibility, I guess.
My point is that it doesn't really help US in this situation if we try to enforce it. Other companies can guarantee stuff like "If you only use our API interface, your stuff won't break for at least two major releases!" - we can't. A lot of our changes also affect public interfaces.
My other point was that our public interfaces only cover what WE needed as an interface. Not what third-party content developers needed. Sometimes our requirements will cover their's, sure. But if you really, really need a bow that reloads faster on Wednesdays, you will most likely have to "hack" the old bow in some way and change some internals.
And hey! When you do that with an
appendto
, you won't have to care about PrivateCall
anyway, since the script becomes part of the original object and is thus allowed to access variables etc. anyway.So I think the best we can do is give a lead on how we intended the methods to be used (aesthetical
private/public
). And if the lead is "I didn't really plan this to be accessed from outside" then the one hacking the object has to figure out themselves if they really need to change/call that method anyway...
-------------------------
Non-global methods that are part of a public interface and intended to be called from other objects or scripts, should be prefixed with the
public
keyword. As a part of the public interface of an object public
methods should be documented with comments accordingly.Non-interface methods should be prefixed with the
private
keyword. Do not leave out the access specifier. The keyword protected
is deprecated.-------------------------
?
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill