Spaghetti code edorsement?

General feedback and discussion of the game.

Moderator: Forum Moderators

User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

Espreon wrote:It doesn’t have to be an insult, and bumbadadabum wasn’t using it as one.
He wasn't the only one who used it, that's right. He didn't mean it as an insult, but it is usually recongised as an insult.
Espreon wrote:Worthwhile editors, such as Notepad++, have grep-like features built-in.
It isn't much know for beginners, maybe even less than the findstr command. To be honest, I find it more comfortable to use Notepad++ even on Linux when grep is available.
Espreon wrote:How does it spread such beliefs? I doubt he said it makes things faster.
Many people wonder why are there so many of these macros. And why are these things moved into additional files. The core macros are fine, they are used in many scenarios, and some of them also serve as useful WML fragments, but this can't be said about this. I initially thought, and many other people thought too, that they are like functions in C/C++/Basic/Lua/Java/Pyhton/whatever. Shadowmaster once wrote this: 'WML, as opposed to popular belief, is not parsed on the fly; it is all already parsed and processed in memory.' Dunno had problems resulting from this belief. What would be the source of this belief if not macro overusage? I have learned that it was wrong when I learned to read the contents of save files.
Espreon wrote:Our philosophy is to separate things as much as possible and within reason, for we like to make things more manageable.
I doubt it helps you manage it better. Extra files slow down when copying, when checking revisions, when loading and in many other cases. These files are actually edited pretty rarely.
Espreon wrote: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.
There are relatively good tutorials, but back in the day when I was learning WML, I wasn't able to find it. I got to the wiki, but I could not find any relevant information about WML anyway, so I learned most of it by reading existing codes and experimenting.
User avatar
vultraz
Developer
Posts: 960
Joined: February 7th, 2011, 12:51 pm
Location: Dodging Daleks

Re: Spaghetti code edorsement?

Post by vultraz »

Dugi wrote:There are relatively good tutorials, but back in the day when I was learning WML, I wasn't able to find it. I got to the wiki, but I could not find any relevant information about WML anyway, so I learned most of it by reading existing codes and experimenting.
Your own code is still very inelegant and non-noob-friendly. Perhaps you should work on improving your own work before urging others to do so.
Creator of Shadows of Deception (for 1.12) and co-creator of the Era of Chaos (for 1.12/1.13).
SurvivalXtreme rocks!!!
What happens when you get scared half to death...twice?
User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

Your own code is still very inelegant and non-newbie-friendly. Perhaps you should work on improving your own work before urging others to do so.
I am aware of it and I plan to add comments explaining it (still, I usually define macros in scenarios/files where they are used and place them in other files only if they're used in many scenarios/files). But it is a recently completed add-on, not a mainline campaign that should be an example (and I am not asking you to write it somehow, I am complaining against the changes that make it worse that you're purposefully making).

Many things are inelegant simply because if I wrote it normally, it would be unplayable. If I kept the functionality, but gave up all this imagined WML, [instert_tag] usage, complex variable system and bizarre events and base it all on macros, it would eat tens of gigabytes of RAM, show 2 frames per second and generate 500 megabytes large save files.
User avatar
Pentarctagon
Project Manager
Posts: 5564
Joined: March 22nd, 2009, 10:50 pm
Location: Earth (occasionally)

Re: Spaghetti code edorsement?

Post by Pentarctagon »

Locked at Dugi's request.

Misread the report, sorry :oops:

On a related note, I'd like to remind everyone to stick to arguing about what is actually better or worse, not denigrating other people's coding style.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
shevegen
Posts: 497
Joined: June 3rd, 2004, 4:35 pm

Re: Spaghetti code edorsement?

Post by shevegen »

The problem with using XML as code-like construct is that it will always be inavoidably hard to read, maintain, extend and use.

There is no point in arguing - WML is ugly. What alternatives are there? You can "optimize" the existing WML but you still stay within the awful constraints of the system. And this is not only confined to WML either - look at the codebase of Wesnoth, the inherent need to "extend" all those little nice things with boost, and today's amount of megabytes required to use Wesnoth.
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.
How does this help him at all? You completely sidestep the issue in this classical bikeshedding approach.

You can paint the bike black or white, but it still remains a crappy bike.

I abandoned the XML landscape years ago and never regretted that decision.
User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

I am not suggesting to use XML, just telling that it can be parsed pretty fast and that WML might be parsed almost as fast, if there wasn't so many files and macros - if a bugged add-on generates a 70 megabytes' save file (that is more than the core WML, but in a single file), it can read the archive and completely parse it within seconds.
Pentartctagon wrote:On a related note, I'd like to remind everyone to stick to arguing about what is actually better or worse, not denigrating other people's coding style.
This is sort of unavoidable, as I am complaining that a certain coding style is worse.
User avatar
Turuk
Sithslayer
Posts: 5283
Joined: February 28th, 2007, 8:58 pm
Contact:

Re: Spaghetti code edorsement?

Post by Turuk »

Dugi wrote:This is sort of unavoidable, as I am complaining that a certain coding style is worse.
Someone can indicate why they think their way is better than someone else's without using tone/language that is degrading. That applies to everyone. If this thread is unable to have a reasonable discussion on merits of one style over another without hostility, than it doesn't need to be open.
Mainline Maintainer: AOI, DM, NR, TB and THoT.
UMC Maintainer: Forward They Cried, A Few Logs, A Few More Logs, Start of the War, and Battle Against Time
User avatar
Coffee
Inactive Developer
Posts: 180
Joined: October 12th, 2010, 8:24 pm

Re: Spaghetti code edorsement?

Post by Coffee »

To my knowledge none of the developers endorse spaghetti code.

There's currently LUA for speed and complex stuff and WML for easy to get started coding with a great reference guide.

If WML replays and such start to spiral out of control in file size it's likely that they will eventually be optimized by one or more of the developers. You could do this by, say, manually compressing the file into a series of bytes that represent each [WML tag] entry index and converting to some sort of bitstream. There have been ideas floated about, but IMO it is not currently a big concern.

In open source you often see plain text file formats with sparse code or data. This is probably the best way to not lock out features through too soon implemented optimizations.

Myself, I am a big fan of WML macros being implemented in core as efficiently as possible. Macros such as IF_VAR. In my own UMC I had to make many macros such as ELSE_IF_VAR, CASE_IS and so on, to make it easier for me to understand. Also, the event model seems to cut down on macro expansion 'spaghetification' in practise.

My 2 cents.
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Spaghetti code edorsement?

Post by iceiceice »

The term spaghetti code refers to the practice of using obscure control structures to make a program excessively complicated. It is a pejorative so of course no one will endorse it. If you think UtBS suffers from too much subdivision into tiny pieces, it might make more sense to accuse them of making Ravioli code. :P
Coffee wrote: Myself, I am a big fan of WML macros being implemented in core as efficiently as possible. Macros such as IF_VAR. In my own UMC I had to make many macros such as ELSE_IF_VAR, CASE_IS and so on, to make it easier for me to understand.
If ELSE_IF_VAR works like I imagine, it should definitely be a core macro. Better to get everyone using these things than to have everyone develop slightly different versions of the same syntactic macros.
Coffee wrote:If WML replays and such start to spiral out of control in file size it's likely that they will eventually be optimized by one or more of the developers. You could do this by, say, manually compressing the file into a series of bytes that represent each [WML tag] entry index and converting to some sort of bitstream.
Won't gzip pretty much do that automatically anyways?
User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

Yes, I meant rather ravioli code than spaghetti code. I didn't complain against macros to replace repetitive syntax blocks like {IF_VAR} (though I am not using them), rather against the usage of macros to replace parts of code used only once, in one file, that is not too long.
User avatar
Alarantalara
Art Contributor
Posts: 786
Joined: April 23rd, 2010, 8:17 pm
Location: Canada

Re: Spaghetti code edorsement?

Post by Alarantalara »

Under the Burning Suns is a rather unfortunate example. It's very old and like most mainline campaigns, has generally only received the minimum amount of attention to keep working (unsurprisingly, if something is working, it's unlikely that someone will fix it just because it could be better). I wouldn't be surprised if some of what you complain about is because when it was written, you couldn't write it better.

For example, if you look through the 1.10 version, you'll find references to green trolls when the last time any troll was green was 1.2 (and most were gray then), terrain changes that were only needed in the old single character terrain syntax, and little to no attempt to take advantage of new WML features. I encourage you to compare 1.11 to 1.10 and even then, there is still room for improvement.
User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

My problem was horrendous macro usage (which is the same since very long), not avoiding using some newer WML tags or structures. I don't know whether it was like that from the beginning, but I was shocked how bumbadadabum was defending it, especially if it increases the loading time (and I believe that is even worse for human reading).

Nobody reads threads from the beginning or what?
User avatar
Alarantalara
Art Contributor
Posts: 786
Joined: April 23rd, 2010, 8:17 pm
Location: Canada

Re: Spaghetti code edorsement?

Post by Alarantalara »

Macro usage and new WML are related. Nested and custom named events are new from the point of view of this campaign. It was originally written with macros only because there was no choice.
User avatar
Dugi
Posts: 4961
Joined: July 22nd, 2010, 10:29 am
Location: Carpathian Mountains
Contact:

Re: Spaghetti code edorsement?

Post by Dugi »

Can you please read the initial post? No custom or nested events are involved.
This is not a quote:
I saw that many code parts that are used only once in a single file 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 from scenario 1 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, as if the macros could be easily reused in another campaign.
User avatar
Alarantalara
Art Contributor
Posts: 786
Joined: April 23rd, 2010, 8:17 pm
Location: Canada

Re: Spaghetti code edorsement?

Post by Alarantalara »

I have read the initial post several times.

Kaleh's abilities are currently included as a file at the end of every scenario so make more sense as a separate file (but should really be moved to his unit definition at which point they would go into the same abilities file as the others). Including events in a unit definition wasn't possible either when this campaign was first written.
The WML for the protagonists is repeated in every scenario (to make testing easier) - they aren't returned in later scenarios using recall.
Thus neither of these meets your criteria for one-off macros.

As for the dehydration code, it's 349 lines long and is a self-contained and logically separate unit. Given that it's not directly related to the rest of the scenario, it's probably easier to understand there instead of buried somewhere in a 2000+ line file.

As a result, when I think of macros in the campaign, I think of the ones within the scenarios that generate the scenario bloat you mention, which are the ones that are fixed by custom events.

Edit: I should mention that I'm not personally a fan of the UTBS_INCLUDE macro mentioned in the linked thread, which as far as I can tell was added with the intention of making forking to UMC easier in exchange for making the code more complicated. It's a far better example of your problem with the campaign. It also made the placement of Kaleh's abilities in a separate file make less sense. Before it was added, Kaleh's abilities were simply included as a file without using a macro and thus made sense as a separate file. With the UTBS_INCLUDE macro, that reason is now gone (but of course wasn't fixed due to the same minimal update policy I mentioned already).
Locked