Spaghetti code edorsement?

[Dugi]

I have come to a discussion with bumbadadabum about this topic, and I found his opinions greatly dogmatic and confused. I can't know what is the general opinion, but I know that my opinions about wesnoth's bowels greatly differ from his opinions.

I was quite displeased by some ways the game's WML is written, in this case I'll use some structures in Under the Burning Suns as examples.

I have learned WML from reading and imitating scenario codes. I first understood the side and unit definitions, then I understood the effects of direct actions. It took me some time to fully understand the variables there (I knew how to write simple programs in Basic already, but the event-defined variables were confusing). Frequently I just remembered in which scenario I saw something similar to the thing I was trying to code, and then I used the existing solution, experimenting with it to find out how far can I change it. Then I could go deeper and deeper with experimentation. I found the wiki quite useless to learn the basics, frequently it's impossible to tell where is the information I seek, and only when I learned it all from existing files I was able to learn or look up everything I needed there. Clearly written example codes are essential for learning.
I have learned the basics of C++ in a similar way.

However, many of the structures I saw in this UtBS are the exact opposite. I saw that many code parts that are used only once are placed into a separate files in the utils folder. Kaleh's abilities aren't in the abilities.cfg file (where everyone looks up abilities), but in a separate file (both of them are far from being too long). The SingleUnitWML properties of protagonists are replaced by macros defined in a separate file, where they are together. Scenario 2 specific thirst is placed in a separate file in utils too.

Creating separate files for everything and macro overusage makes the code less understandable (macros aren't obvious links to files, and not every newbie understands what they are and that some of them are campaign-specific). If a macro is used only in one file, why isn't it defined at the top of the file or before the location where it is used (and undefined just bellow in the case of perfectionism)? If a piece of code is used only once, what is the reason to use a macro if the file where it would be isn't too large?
This way, pieces of code are spread into many locations, and it isn't easy to find them all. Even if you find them all, reading the code requires to remember the contents or to switch between reading on many locations. Additionally, not all newbies understand what are the macros.

The consequences of macro overusage are known, read about Dunno's issues for example. Heindal is still facing similar problems (though he's busy elsewhere now). The belief that macros make the game faster is dangerous, and using many macros leads to use of macros replacing large pieces of code many times inside other macros that are used many times (I've seen add-ons that had several times less code than mine and took several times more time to load). Also, some add-ons' scenarios are almost impossible to read because many parts of them are in some files in utils and very hard to find, and I am quite sure obfuscation wasn't intended.

[Paulomat4]

I basically agree with dugi. As a fresh wml learner kaleh's amlas posed a similar problem to me, and i had to learn it from somewhere else. On the other hand, i find the wiki quite helpful and can't imagine myself coding without it opened.

[artisticdude]

So what I'm getting out of this is that there's some disagreement concerning the current organization philosophy used for WML in UtBS. There are some nitpicks I have with the organization myself, but a lot of the current "problems" may be at least partially due to the campaign's pre-mainline legacy setup that no one ever felt was terrible enough to warrant fixing. To be honest, I can't imagine having noob-friendly WML was ever very high on UtBS's development priority list.

The belief that macros make the game faster is dangerous, and using many macros leads to use of macros replacing large pieces of code many times inside other macros that are used many times (I've seen add-ons that had several times less code than mine and took several times more time to load).

Yeah, macros are basically runtime text substitutions. As far as I know, they really shouldn't have any significant impact on performance either way. One thing that does strike me is that organizing WML into multiple files without any real reason to do so would mean the game has to read more files from disk, and disk reading from multiple separate files can be a very expensive operation in terms of performance. But then, I have a limited (and possibly outdated now) understanding of how Wesnoth handles these operations, so I can't comment on the matter with any real authority.

[Dunno]

I agree with your rant but I don't really know what you're aiming for

Do you want to start a project of rewriting and optimising mainline campaigns or something? Or is it just your personal appeal to umc creators?

[iceiceice]

Creating separate files for everything and macro overusage makes the code less understandable (macros aren't obvious links to files, and not every newbie understands what they are and that some of them are campaign-specific). If a macro is used only in one file, why isn't it defined at the top of the file or before the location where it is used? If a piece of code is used only once, what is the reason to use a macro if the file where it would be isn't too large?

Offhand, one reason might be an extensible design. They might only be used once right now but extensible design means you anticipate that it might be used in other places in the future. I don't think that having a macro definition in a separate file and then using it only once necessarily indicates bad design as you suggest.

Also, some add-ons' scenarios are almost impossible to read because many parts of them are in some files in utils and very hard to find, and I am quite sure obfuscation wasn't intended.

Maybe when their add-ons get to a certain size people should be aware they need to make a README file that helps the casual reader to find things.

[Dugi]

I am not asking for any refactoring or rewriting, or asking others to allow me to do it, I just don't like the direction where this is going. bumbadadabum (who believed he represents the developing staff) told me this structuralisation of WML this is what is actually preferred. I disagreed with it, but he wasn't very communicative and didn't have the power to change it anyway. So I want to discuss this matter openly, to know the general opinion and to present it to the developers. I merely want to change the direction where the rewrites of mainline WML are going.

may be at least partially due to the campaign's pre-mainline legacy setup that no one ever felt was terrible enough to warrant fixing.

The complicatedness of Kaleh's code Paulomat4 complained about isn't the problem I see here, because there is no other way to achieve that functionality. UtBS needs complex codes to achieve the functionality it has at the moment, my problem is just that it shouldn't be moved to more files using macros to make it even more complicated.

Yeah, macros are basically runtime text substitutions. As far as I know, they really shouldn't have any significant impact on performance either way.

That's the problem. They aren't runtime text substitutions. They are substituted when the campaign/multiplayer/save file is loading. Then they dwell in the RAM fully substituted. This difference is not very large, but if they were substituted in runtime, using many macros in other macros that are many times in other macros wouldn't result in problems with huge save files and RAM consumption. Some people face severe problems due to this belief. I also believe that macros take some time to look up, but that isn't the main problem, macros are frequently needed.

One thing that does strike me is that organizing WML into multiple files without any real reason to do so would mean the game has to read more files from disk, and disk reading from multiple separate files can be a very expensive operation in terms of performance. But then, I have a limited (and possibly outdated now) understanding of how Wesnoth handles these operations, so I can't comment on the matter with any real authority.

This is perfectly right. Accessing many files was always a problem, because it requires to ask the OS to give it access to the file, the OS reads if the program can have access to that file, then the file is added to the program as file stream and can be opened. The number of files Wesnoth loads is probably the main reason why it takes so much time to load (I have seen an XML parser that would be able to parse all wesnoth code in less than a second if it was in XML and it could read t quickly enough, so the time the parsing takes isn't the reason probably, and normal disks can be read with a speed of about 40 megabytes per second). Most modern games solve this by creating archives where all small files are stored (and archives like .zip are read and decompressed way faster than many small files).

Offhand, one reason might be an extensible design.

But UtBS is not going to be extended. There is a reason to put abilities into one file, but not into two files. And it does not explain why the scenario-specific dehydratation code is separated into another file.

Maybe when their add-ons get to a certain size people need to realize they need to make a README file that tells reader where to find things.

Resource packs usually have this and I believe I will get myself to write such file also for my campaign, but mainline campaigns don't. It would be a nice idea if they had, regarding the didactic value newbies seek in them.

[iceiceice]

Offhand, one reason might be an extensible design.

But UtBS is not going to be extended. There is a reason to put abilities into one file, but not into two files. And it does not explain why the scenario-specific dehydratation code is separated into another file.

Maybe the intention of the author was that other scenarios and campaigns will cite these files within the UtBS campaign folder?

Maybe when their add-ons get to a certain size people need to realize they need to make a README file that tells reader where to find things.

Resource packs usually have this and I believe I will get myself to write such file also for my campaign, but mainline campaigns don't. It would be a nice idea if they had, regarding the didactic value newbies seek in them.

Yeah that's a bit unfortunate. It would be nice I guess if authors / people who are familiar with the campaign/add-on files would go back and add some readme files saying just generally what is where. This would probably take much less work than back commenting all the code; not that its a replacement but at least a good start, and very helpful.

[Dugi]

Maybe the intention of the author was that other scenarios and campaigns will cite these files within the UtBS campaign folder?

Well, it was actually taken into Invasion from the Unknown, but Shadowmaster was certainly far from needing advice how to code that himself. But there was no explanation how to use all these macros (some of them were used in other macros there, but some were to be used in unit codes etc).

[Telchin]

I admit that in my campaign I often use macros that are used only in a single scenario (and often only once). This is partially because it's my first campaign and I was sort of practicing how macros work usually, but also because sometimes I felt it actually made the code easier to understand. Take for example this code:

Spoiler:

Code: Select all

#define LICHEN_ONE
 {QUAKE "rumble.ogg"}
[terrain]
	terrain=Uu^Uf
	x=14
	y=40-44
[/terrain]
[terrain]
	terrain=Uu^Uf
	x=15
	y=40
[/terrain]
[terrain]
	terrain=Uu^Uf
	x=16
	y=39
[/terrain]
[terrain]
	terrain=Uu^Uf
	x=17
	y=40
[/terrain]
[terrain]
	terrain=Uu^Uf
	x=18
	y=40-44
[/terrain]
[terrain]
	terrain=Urb
	x=19
	y=41-45
[/terrain]
[terrain]
	terrain=Urb
	x=15
	y=41-44
[/terrain]
[terrain]
	terrain=Urb
	x=17
	y=41-44
[/terrain]
[terrain]
	terrain=Urb
	x=16
	y=40-44
[/terrain]
[terrain]
	terrain=Rp^Vhcr
	x=16
	y=42
[/terrain]
[capture_village]
	side=2
	x=16
	y=42
[/capture_village]
[terrain]
	terrain=Wwg
	x=14
	y=35-39
[/terrain]
[terrain]
	terrain=Wwg
	x=15
	y=35-39
[/terrain]
[terrain]
	terrain=Wwg
	x=16
	y=34-38
[/terrain]
[terrain]
	terrain=Wwg
	x=17
	y=35-39
[/terrain]
[terrain]
	terrain=Wwg
	x=18
	y=35-39
[/terrain]
[terrain]
	terrain=Wwg
	x=13
	y=42-43
[/terrain]
[redraw]
	side=1
[/redraw]
#enddef

That's lot of terrain tags, isn't it? And it only changes the terrain on a part of the map in a single scenario. Said scenario has multiple similiar changes (hence the "ONE" in the macro's name) happening at different times as well as another cycle of macros creating enemy units. If I just wrote them in the scenario's file it would become a confusing mess of [terrain] tags alternating with [unit] tags, as each wave of terraforming/enemy reinforcements is very similar, but still too differrent to use one macro for all of them. Instead I turned these walls of text into macros and shoved them into a separate file(s). The scenario proper then has this:

Spoiler:

Code: Select all

#Every 2 turns Ravyan summons more fungus. Actual terrain modifiers are in utils/lichen.cfg, here are only dialogues:
[event]
	name=turn 3
	[message]
		speaker=Ravyan
		message= _ "Rise up! Rise from the sea bottom!"
	[/message]
	 {LICHEN_ONE}
	 {SEA_HEAD_STUCK}
[/event]

Note #1: On the upside, I mention which file the macros are hidden in. This should alleviate the issue of macros not being where people might look for them.
Note #2: On the downside, my names for files which store the macros are not always descriptive. In the example above the macro LICHEN_ONE is in the file lichen.cfg, but I'm not always that sensible. Hopefully my comments mentioned in Note #1 should always steer potential code-readers to the correct file. (Somebody who downloaded my UMC and has lots of spare time might check it.)
Note #3: While my example hid repetitive code and left the dialogues in the scenario, this i not always case for other UMCs. The White Troll actually uses macros for all its dialogues and puts them into a single file. This might seem like a horrible abuse of macros from Dugi's perspective, but presumably also makes the campaign easier to spellcheck and/or avoid continuity issues. (Can any of said campaign's creators elaborate on this decision?)

[Dugi]

Well, in the case of the example you are showing, it is a case where you need to move some clutter out of the scenario, which is the case where the scenario file would be too long or unreadable because of being filled with clutter. It could be probably done using some random functions instead of macros for better replayability. But this is an extreme case.

What you are mentioning about The White Troll, it is another case where confusing macro usage is reasonable. It makes the code pretty confusing, but text revisioners will enjoy it (many of them are asking for all texts in a single file, ideally a .docx file without WML markup).

By the way, remember about the spaghettiness in the UtBS thirst code? I have just found somebody having problem with using it.

[Telchin]

Yes, I've just replied to that thread (my 250th post!) and actually linked this thread, as his (her?) problem was the use of UtBS code. When looking into the coresponding UtBS files I realized that it uses a macro defined in UtBS's _main.cfg ! (The macro is actually used later in the _main.cfg file, which is probably the reason for this macro abuse, but it also proves your point about UtBS' macros being confusing for newbies.)

[grakksilpex]

I am a His just in Case xd. But I admit im a newbie but i still do my efforts to use macros but the macro problem im having in the campaign is still active and i can't manage to make it work again. If You have any more ideas just let me know.

http://forums.wesnoth.org/viewtopic.php ... 29#p561629

[Astoria]

Let me start off by saying it has always been recommended for newbies to NOT look at UtBS's coding. Also, I never said I was speaking for the entire development team. I only spoke for the team currently working on Eftboren.

The code in UtBS isn't meant to be noob-friendly. Also, the campaign itself doesn't really facilitate such code. The usage of macros in it only makes it easier to read in my opinion, and the complaint about macros being in other files seems completely unreasonable. The macros are usually in logical files, and and easily found. Also, if you can't seem to find the file the macro is in, grep is an easy to use command.

The thing you describe as "Spaghetti code" and "macro overusage" is what I believe is the more elegant, better way of doing things. I am not preaching a dogma; this is what I believe, and that opinion is one I, for the most part, share with the others working on Eftboren. It's indeed not the most noob-friendly way, but I don't think noob-friendliness should be the first priority with the most complex mainline campaign. There is already plenty of noob-friendly material in mainline. If we disregard noob-friendliness, we can focus more on making the code more elegant and functional for more experienced coders, as well as making the code more flexible and just better in general.

Of course we (Dugi and myself) think about this a whole lot different, I think there is, and there should be, room for both.

[Dugi]

Let me start off by saying it has always been recommended for newbies to NOT look at UtBS's coding. Also, I never said I was speaking for the entire development team. I only spoke for the team currently working on Eftboren.

Maybe you could clarify who is in that team and what Eftboren is. And please keep on mind that I was not complaining about your changes, these changes were there already in 1.10, but you were defending them.

The code in UtBS isn't meant to be noob-friendly.

Did you know that noob is an insult? You're speaking about newbies.

According to Urban Dictionary:

1. A noob is a person who really sucks at a game but refuses to learn/listen to people who are skilled. Many of them may have been playing the game for a while, but still suck at it. They usually have no hope.

2. Noobs generally get extremely upset over being beaten and can not admit that they are bad at the game. Often they will accuse the skilled people of hacking.

3. This word is an insult in general so people often wrongly use it and just call somebody that they do not like a noob, regardless of skill.

4. This term is often confused with the term "newb," and people who confuse these terms are usually either noobs themselves, or somebody who is good but is just mean (and has a lot of pride for being skilled).

Newb
1. Newb comes from "newbie." Somebody new to a game and they will generally suck at it. However, the reason that they suck is because of their unfamiliarity to the game. They have the potential to become good. This is not a derogatory term.

2. Also, newbs are not always bad at the game, as they may have played similar games in the past, but when it comes to knowing where things are, or information- they don't know.

Noob
1. Player: Hey man you keep getting killed, I'd recomend using a sniper rifle in this large zone, rather than that shotgun.
Noob: dude stfu i no wat im doin *gets killed again*

2. *noob gets killed yet again*
Noob: WTF u freaking hacker u suck
Player: Err...No I've just had the game for quite a while.
Noob: STFU
Player: .....Alright I have had enough of you, noob. *begins to kill the noob specifically over anybody else*

3. *Player1 spawns and picks up a shotgun*
Player2: wtf taht shotgun shuld be minez i was rite there 2 pick it up.
Player1: Sorry, I guess I got it.
Player2: u freaking noob i hate u.
Player1: Dude, we are on the same team, it benefits you as well.
Player2: SCREW U NOOB!!!111

4. Newplayer: Hey where is the enemy base?
Skilledplayer: ROFL NOOB
Newplayer: err....sorry I'm new.
Skilledplayer: LOL at you!!!!112@@
Newplayer: ....

Newb
1. Player: Hey man you really need to work on your shooting. You don't seem to know where you are going either.
Newplayer: ...Yeah I just got the game....sorry.
Player: Yeah, I remember back when I was a newb. You'll get better, and learn where things are.

2. Newplayer: Hey where is the enemy base?
Player: Wow, I saw you shooting pretty well. How could you not know that?
Newplayer: I just got the game, sorry. I'm kind of a newb.
Player: But damn you shoot well.
Newplayer: Yeah, I played game X before, so I have ex...

The usage of macros in it only makes it easier to read in my opinion, and the complaint about macros being in other files seems completely unreasonable. The macros are usually in logical files, and and easily found. Also, if you can't seem to find the file the macro is in, grep is an easy to use command.

More files take more time to load. It may be in a location where you would seek it if you knew it is in a different file, but scenario-specific macros usually aren't (but locating it takes some time anyway). grep is very useful for that, but we all know that it does not exist on Windows. There is a replacement, but it's not much known and does not have all the abilities grep has. Also bear in mind that usual Windows users don't know to use the command line, because it is better and better hidden in later version of Windows. Also most Macintosh 10 users have no idea that there is some Terminal.

The thing you describe as "Spaghetti code" and "macro overusage" is what I believe is the more elegant, better way of doing things.

I have said that you believe that spaghetti code of this kind is better. I disagree, and I have presented my arguments for it. These arguments have been approved as reasonable. You have presented one vague argument. You are preaching that it is better despite the facts that it is less newbie-friendly, slower and it propagates the belief that macros quicken the game.

[Espreon]

Maybe you could clarify who is in that team and what Eftboren is. And please keep on mind that I was not complaining about your changes, these changes were there already in 1.10, but you were defending them.

The Eftboren development team is comprised of bumbadadabum, vultraz, and me. Though, I should note that Alarantalara was also involved for a while.

Eftboren (in Modern English: reborn) is a project that attempts to greatly improve Under the Burning Suns in many ways; as byspel, it takes on the task of rehauling the desert elves.

Did you know that noob is an insult?

It doesn’t have to be an insult, and bumbadadabum wasn’t using it as one.

More files take more time to load. It may be in a location where you would seek it if you knew it is in a different file, but scenario-specific macros usually aren't (but locating it takes some time anyway). grep is very useful for that, but we all know that it does not exist on Windows. There is a replacement, but it's not much known and does not have all the abilities grep has. Also bear in mind that usual Windows users don't know to use the command line, because it is better and better hidden in later version of Windows. Also most Macintosh 10 users have no idea that there is some Terminal.

Worthwhile editors, such as Notepad++, have grep-like features built-in.

I have said that you believe that spaghetti code of this kind is better. I disagree, and I have presented my arguments for it. These arguments have been approved as reasonable. You have presented one vague argument. You are preaching that it is better despite the facts that it is less newbie-friendly, slower and it propagates the belief that macros quicken the game.

How does it spread such beliefs? I doubt he said it makes things faster.

Our philosophy is to separate things as much as possible and within reason, for we like to make things more manageable.

Instead of complaining about the way we do things, why don’t you focus on improving the wiki by adding good byspels and tutorials so that neophytes can flourish more easily.