Coding Conventions

The place to post your WML questions and answers.

Moderators: Forum Moderators, Developers

Forum rules
  • Please use [code] BBCode tags in your posts for embedding WML snippets.
  • To keep your code readable so that others can easily help you, make sure to indent it following our conventions.
fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Coding Conventions

Post by fabi » April 14th, 2016, 11:49 am

I like to know every irregularity in the WML api,
namely violations to the Coding Conventions.

Things like canrecruit= which should be can_recruit= or [advancefrom] which should be [advance_from]

A second category are attributes which don't match the content or type of the value.

Code: Select all

[attack]
    name="bow"
    description= _ "bow"
    ...
[/attack]
attack.name is not a translatable name but the id used for filtering animations.
While attack.description is not a description but a single translatable name.

So I think it should be:

Code: Select all

[attack]
    id="bow"
    name= _ "Fine Bow"
    description= _ "Bows are far better than axes."
[/attack]
So far I know about:

movetype.name should be 'id'
canrecruit
[advancefrom]
team_name -> not translatable, should be 'team_id' ?
user_team_name
'name' in the event tag -> not translatable should it be event 'type' or 'id'?
moveto -> move_to
prestart -> pre_start
prerecall -> pre_recall (and similar) ..
Please help with "and similar", I am not a native English speaker and thus not sure which words are valid.
[allow_extra_recruit] extra_recruit= -> allow_extra_recruit.type
the attack id/name/description issue

Please tell me when you discover more of those and help me make the list complete.
Thank you ^_^
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

User avatar
skeptical_troll
Posts: 425
Joined: August 31st, 2015, 11:06 pm

Re: Coding Conventions

Post by skeptical_troll » April 14th, 2016, 12:09 pm

A few I can think of (if I understand what you mean):

team_name -> not translatable, should be 'team_id' ?
'name' in the event tag -> not translatable should it be event 'type' or 'id'?

moveto -> move_to ?
prestart -> pre_start ?
prerecall -> pre_recall (and similar) ..

Something I also find confusing is that [allow_recruit] uses 'type' to specify the new units and [allow_extra_recruit] has 'extra_recruit'

fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Re: Coding Conventions

Post by fabi » April 14th, 2016, 12:11 pm

skeptical_troll wrote:A few I can think of (if I understand what you mean):

team_name -> not translatable, should be 'team_id' ?
'name' in the event tag -> not translatable should it be event 'type' or 'id'?

moveto -> move_to ?
prestart -> pre_start ?
prerecall -> pre_recall (and similar) ..

Something I also find confusing is that [allow_recruit] uses 'type' to specify the new units and [allow_extra_recruit] has 'extra_recruit'
Yeah, all good catches.

Thank you very much.
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

User avatar
Ravana
Forum Moderator
Posts: 2244
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: Coding Conventions

Post by Ravana » April 14th, 2016, 12:56 pm

I find it quite irregular how some tags want [filter] and some only want its content.

fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Re: Coding Conventions

Post by fabi » April 14th, 2016, 12:59 pm

Ravana wrote:I find it quite irregular how some tags want [filter] and some only want its content.
Okay, let's fix that by just allowing filter everywhere (edit: everywhere a SUF is expected).
This is also backwards compatible.

(And I need to do that anyway for different reasons)
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

User avatar
beetlenaut
Developer
Posts: 2390
Joined: December 8th, 2007, 3:21 am
Location: Washington State
Contact:

Re: Coding Conventions

Post by beetlenaut » April 14th, 2016, 9:14 pm

skeptical_troll wrote:prestart -> pre_start ?
prerecall -> pre_recall (and similar) ..
Actually, this is wrong. "Pre" is not a word, so it would never be used with a space before the word it modifies. That makes an underscore inappropriate too. Out of prestart, prerecall, and prerecrut, only prestart appears in the dictionary. When we invent our own words by attaching "pre" to other words, we usually use a hyphen, but it's not strictly necessary. It wouldn't help make WML more consistent either, so we should just keep "prerecall" and "prerecruit" as they are, along with anything else that starts with "pre".

I vote for "type" in event. It makes the most sense. Ravana's suggestion is good too. That inconsistency has bothered me for a long time.
Campaigns: Dead Water,
The Founding of Borstep,
Secrets of the Ancients,
and WML Guide

User avatar
skeptical_troll
Posts: 425
Joined: August 31st, 2015, 11:06 pm

Re: Coding Conventions

Post by skeptical_troll » April 14th, 2016, 9:28 pm

beetlenaut wrote:Actually, this is wrong. "Pre" is not a word
Ops, my fault. I wasn't actually totally sure, because prestart and 'pre advance' (as well as 'post advance') are treated in different ways, and both should be written with an hyphen. I guess then the latter should change?

Another thing I find confusing is the [terrain] tag, when you want to filter for the old terrain. Wouldn't be easier to use the keyword 'new_terrain' instead of 'terrain' to specify the new type and 'terrain' for the one to be filtered as in the SLF? At the moment it is a bit counter-intuitive how it works. Just a suggestion..

another one: [endlevel] -> [end_level] ?

gfgtdf
Developer
Posts: 1190
Joined: February 10th, 2013, 2:25 pm

Re: Coding Conventions

Post by gfgtdf » April 14th, 2016, 9:34 pm

Actually, this is wrong. "Pre" is not a word, so it would never be used with a space before the word it modifies. That makes an underscore inappropriate too. Out of prestart, prerecall, and prerecrut, only prestart appears in the dictionary. When we invent our own words by attaching "pre" to other words, we usually use a hyphen, but it's not strictly necessary. It wouldn't help make WML more consistent either, so we should just keep "prerecall" and "prerecruit" as they are, along with anything else that starts with "pre".

I vote for "type" in event. It makes the most sense. Ravana's suggestion is good too. That inconsistency has bothered me for a long time.
We also have the "pre_advance" event, the reason why the underscroe was added is becasue the "post_advance" event had that too.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.

User avatar
beetlenaut
Developer
Posts: 2390
Joined: December 8th, 2007, 3:21 am
Location: Washington State
Contact:

Re: Coding Conventions

Post by beetlenaut » April 14th, 2016, 9:45 pm

The "post" prefix doesn't use a space either. I'm sure the reason someone added an underscore is that they felt that something should go there since they were inventing a word, but it should be a hyphen if anything.
Campaigns: Dead Water,
The Founding of Borstep,
Secrets of the Ancients,
and WML Guide

fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Re: Coding Conventions

Post by fabi » April 14th, 2016, 9:52 pm

Okay, so event names or better types are a special case.

I have reasons to capitalize them.

Start
TimeOver
Turn1
Side2Turn3

And so on.

I am also going to call them
PreStart
PreRecruit
PostAdvance
and so on. Using CamelCase and capitalizing the part after "Pre" or "Post" feels fine.
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Re: Coding Conventions

Post by fabi » April 15th, 2016, 5:01 pm

What about "movetype"?

movetype.name is also used as id.
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

User avatar
Celtic_Minstrel
Developer
Posts: 1532
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Coding Conventions

Post by Celtic_Minstrel » May 5th, 2016, 1:20 am

skeptical_troll wrote: 'name' in the event tag -> not translatable should it be event 'type' or 'id'?
I feel it necessary to point out here that 'id' already has a meaning in [event], so changing 'name' to 'id' is absolutely out of the question.

There are some inconsistencies in format as well as naming - for example, colours are specified in several slightly different ways depending on which tag they appear in.
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.

User avatar
doofus-01
Art Contributor
Posts: 3809
Joined: January 6th, 2008, 9:27 pm
Location: USA, the civilized part.

Re: Coding Conventions

Post by doofus-01 » May 5th, 2016, 3:35 am

fabi wrote:Using CamelCase and capitalizing the part after "Pre" or "Post" feels fine.
How do you determine what feels fine?
BfW 1.12 supported, but active development only for BfW 1.13/1.14: Bad Moon Rising | Trinity | Archaic Era |
| Abandoned: Tales of the Setting Sun
GitHub link for these projects

fabi
Developer
Posts: 1223
Joined: March 21st, 2004, 2:42 pm
Location: Germany

Re: Coding Conventions

Post by fabi » May 9th, 2016, 4:59 pm

doofus-01 wrote:
fabi wrote:Using CamelCase and capitalizing the part after "Pre" or "Post" feels fine.
How do you determine what feels fine?
That is not exactly a precise question, so let me elaborate a little bit more.

I guess most people would answer that determining what feels fine means hearing about some noise in your guts.

In the case of the event "name" (or maybe "type") I have to share some more thoughts.

All of scenario coding is now done below some [event] tags.
I introduce additional syntactic sugar by just taking every capitalized table key as the "name" of an event handler.

All code snippets are equivalent:

Code: Select all

[scenario]
    [event]
        name="prestart"
        [unit]
            type="Elvish Fighter"
            id="Kalenz"
        [/unit]
    [/event]
[/scenario]
-----------------------------------------------------
scenario
    event:
        type: "PreStart"
        command: ->
            unit
                type: "Elvish Fighter"
                id: "Kalenz"
------------------------------------------
scenario
    PreStart: ->
        unit
            type: "Elvish Fighter"
            id: "Kalenz"
My guts tell my that CamelCase is a good naming convention if you already recognize capitalized letters.
Also, whitespace needs to be escaped in Lua table key identifiers.
So all whitespace and underline go in favor of CamelCase.

The last gut thing is that having one "PreSomething" means "PreAnother" is also a good idea.
"Wesnoth has many strong points but team and users management are certainly not in them." -- pyrophorus

User avatar
Celtic_Minstrel
Developer
Posts: 1532
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Coding Conventions

Post by Celtic_Minstrel » May 9th, 2016, 6:13 pm

Underscores are perfectly valid in Lua table key identifiers.
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.

Post Reply