Jump to content

Photo

Script critique and advice thread


  • Please log in to reply
17 replies to this topic

#1 Saffith

Saffith

    IPv7 user

  • ZC Developers

Posted 26 February 2020 - 07:54 PM

Let's try something new here. No idea if it'll work out, but I figure it's worth a shot.

I think one reason scripting seems so hard is that there's a lack of useful guidance. Scripts aren't directly visible in quests, so you don't get much feedback aside from bug reports. Sure, there's a whole forum you can ask, but it's generally used for specific questions; it just doesn't feel like the right place to ask for general help with style and design. This thread is for precisely that. If you've got a script you'd like feedback on but no particular issues, or perhaps questions that feel too general or... let's say "squishy" to give their own thread, post them here.

We should probably have some guidelines...

Script posters
  • Use code tags. Otherwise, the formatting gets screwed up and your script will be hard to read.
  • If you're posting a long script, consider putting it in a hidden tag or linking to something like Pastebin to cut down on scrolling.
  • Unless they're extremely short and simple, comment your scripts. No one else already knows how your script works. You can get away with fewer comments if the code itself is descriptive.
  • Give at least a brief description of what your script does. If you want feedback on how a script performs, not just coding issues, you should include a demo or video. Otherwise, it's nice, but not at all essential.
  • We'll assume you're including std.zh, but if your script uses any other headers, be sure to mention them.
Commenters
  • The main goal here is to help people become better scripters - to get comfortable with the language and logic and to develop good coding practices. Examples of how to do things and explanations of how they work and why they're done that way are helpful.
  • Try not to give the impression that the way you would do something is the best or only way to do it. Except when it really is, of course.
  • As long as they don't actually present problems, don't make too big a deal of things that are matters of personal preference - whitespace usage, naming conventions, things like that.
Everyone
  • Don't assume a script posted here is free to use. If it's not clear, ask first.
Don't worry that a script is too small or too simple to post. It doesn't even have to be a whole script; a function or a code snippet is fine. Or just general questions are okay, too. Anything you'd like input on, go for it.

And don't feel insulted if you get a lot more explanation than necessary. We don't know how much you know, and it may be helpful for others reading.



It seems appropriate to include a script here to get things started. Here's a recreation of the ball shooter near the entrance of the Eastern Palace in LttP, as seen here. It's not too versatile because I wanted to keep it short and simple, but you're welcome to use it if you like.
 
// Ball settings.
const int BSH_LARGE_BALL_COMBO = 888; // This should be a damage combo
const int BSH_LARGE_BALL_CSET = 0;
const int BSH_LARGE_BALL_SFX = 61;
const float BSH_LARGE_BALL_SPEED = 1.5;
const int BSH_SMALL_BALL_COMBO = 889;
const int BSH_SMALL_BALL_CSET = 0;
const int BSH_SMALL_BALL_SFX = 61;
const float BSH_SMALL_BALL_SPEED = 1.5;

// Number of small balls to shoot before shooting a large one.
// If <=1, only large balls are fired.
const int BSH_NUM_SMALL_BALLS = 5;

// Time between balls. Must be at least 1.
const int BSH_SHOT_TIMER = 60;

ffc script BallShooter {
    void run() {
        while(true) {
            // Shoot however many small balls. Wait a bit before each shot.
            for(int ctr=0; ctr<BSH_NUM_SMALL_BALLS; ctr++) {
                Waitframes(BSH_SHOT_TIMER);
                ShootSmallBall(this);
            }
            
            Waitframes(BSH_SHOT_TIMER);
            ShootLargeBall(this);
        }
    }
    
    // Fires a small ball, randomly placed on either the left or right side
    // of the shooter. If there are no FFCs available, does nothing.
    void ShootSmallBall(ffc this) {
        // Get a free FFC. If there aren't any, return without firing.
        int ffcNum=FindFreeFFC();
        if(ffcNum==0)
            return;
        ffc ball=Screen->LoadFFC(ffcNum);
        
        // Set up the ball. No need to keep track of it anywhere;
        // FFCs are automatically cleared when they go too far offscreen.
        ball->Data=BSH_SMALL_BALL_COMBO;
        ball->CSet=BSH_SMALL_BALL_CSET;
        ball->TileWidth=1;
        ball->TileHeight=1;
        ball->EffectWidth=16;
        ball->EffectHeight=16;
        ball->X=Choose(this->X, this->X+16);
        ball->Y=this->Y+16;
        ball->Vy=BSH_SMALL_BALL_SPEED;
        
        Game->PlaySound(BSH_SMALL_BALL_SFX);
    }
    
    // Fires a 2x2 tile ball. If there are no FFCs available, does nothing.
    void ShootLargeBall(ffc this) {
        int ffcNum=FindFreeFFC();
        if(ffcNum==0)
            return;
        ffc ball=Screen->LoadFFC(ffcNum);
        
        ball->Data=BSH_LARGE_BALL_COMBO;
        ball->CSet=BSH_LARGE_BALL_CSET;
        ball->TileWidth=2;
        ball->TileHeight=2;
        ball->EffectWidth=32;
        ball->EffectHeight=32;
        ball->X=this->X;
        ball->Y=this->Y;
        ball->Vy=BSH_LARGE_BALL_SPEED;
        
        Game->PlaySound(BSH_LARGE_BALL_SFX);
    }
    
    // Finds an unused FFC to use for a ball and returns the number of
    // that FFC. Returns 0 if there are none available.
    int FindFreeFFC() {
        for(int num=1; num<=32; num++) {
            // An FFC is considered to be unused if
            // it has no combo or script set.
            ffc f=Screen->LoadFFC(num);
            if(f->Data==0 && f->Script==0)
                return num;
        }
        
        return 0;
    }
}

  • ShadowTiger, Jared, Evan20000 and 3 others like this

#2 Evan20000

Evan20000

    P͏҉ę͟w͜� ̢͝!

  • Members
  • Real Name:B̵̴̡̕a҉̵̷ņ̢͘͢͜n̷̷ę́͢d̢̨͟͞
  • Location:B̕҉̶͘͝a̶̵҉͝ǹ̵̛͘n̵e̸͜͜͢d҉̶

Posted 27 February 2020 - 08:26 AM

I guess I'll contribute. Are pastebin links okay; the forum's edit box broke when I tried to preview it with code tags (apparently too long)? It's not the best commented, but I also don't think it's an unholy abomination either.
https://pastebin.com/fyEM4QvH
There's a few oddities like a couple things in the collision checking Ghost_Z because I was contemplating him have a fat candle leap at one point but decided against it.

https://youtu.be/WxXoUg7egU4
Video showcasing how it works. Feel free to using it as a learning resource if you want, but I request that you not copypaste it until the quest he's in is done.

 

 

EDIT: Uses ghost and maybe a few custom functions I had littered around the script file. I'm terrible at reading the OP and failed to see the header requirement.


Edited by Evan20000, 27 February 2020 - 11:20 PM.


#3 Emily

Emily

    Scripter / Dev

  • ZC Developers

Posted 27 February 2020 - 07:05 PM

Are pastebin links okay;

The answer to this question is almost always yes. Pastebin links are a GODSEND for sharing code bits. Especially when forums/discord can fuck with the formatting; pastebin keeps it pure.



#4 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 27 February 2020 - 11:57 PM

Something that I will suggest, is that whatever style you ultimately use, the best thing to do is to remain as true and uniform to it as possible. Certain elements of style such as casing, indents, comment styles, and identifier formats can all immediately demonstrate what it is at a visual glance with absolutely no level of uncertainty.

 

Being tidy helps, too. Even levels of indentation, and formatting make everything easier to read. Breaking up long statements onto separate lines reduces horizontal scrolling dramatically, making it easier to work with your files.


  • Evan20000 likes this

#5 Saffith

Saffith

    IPv7 user

  • ZC Developers

Posted 28 February 2020 - 01:51 AM

Are pastebin links okay; the forum's edit box broke when I tried to preview it with code tags (apparently too long)?

Good point. Yeah, that's totally fine. I'll edit it in.
 

It's not the best commented, but I also don't think it's an unholy abomination either.
https://pastebin.com/fyEM4QvH

I'm not entirely certain you were actually looking for feedback on that one, but you're going to get it either way, because it's a perfect platform for the main thing I was thinking about when starting this thread: using functions. I think using functions well is a relatively hard thing to learn, because you usually don't really have to.
 
One of the most important uses of functions is to make code more clearly express what it does in the terms you would normally think about it. A function (including run) does one thing, which is often a series of smaller steps. If those smaller steps are also made up of smaller steps, they should generally be put into separate functions. Ideally, the code of a function should match your intuition about how it does what it does.

Look at that ball shooter script above, for example. It's pretty simple, and it could easily be rewritten so the whole thing is in run(). In that case, the logic of run() would look something like this:

Repeat forever:
    5 times:
        Wait one second
        For each FFC:
            If the FFC has no combo or script:
                Remember this FFC as the ball
                Break out of loop
        If a free FFC was found:
            Set the ball's combo, CSet, and size (small ball)
            Put the ball at the shooter's position
            Set the ball's velocity
            Play the shot sound
    Wait one second
    For each FFC:
        If the FFC has no combo or script:
            Remember this FFC as the ball
            Break out of loop
    If a free FFC was found:
        Set the ball's combo, CSet, and size (large ball)
        Put the ball at the shooter's position
        Set the ball's velocity
        Play the shot sound

It works the same, but it's harder than it needs to be to understand what it's doing. All of the implementation details are mixed together, and you have to read them all and mentally sort them out to see what it does.

Split into separate functions as it is, the logic of run() is like this:
 

Repeat forever:
    5 times:
        Wait one second
        Fire a small ball
    Wait one second
    Fire a large ball

It's easier to understand this way not just because it's shorter, but because it's all working at the same level. It's a high-level description of the shooter's behavior; all the details of how shooting a ball actually works are neatly out of the way. It's still doing all the same things, but you don't have to think about them all at once.

The technical term for this is abstraction. The ShootSmallBall function is an abstraction that hides the concrete implementation details.

There is of course some subjectivity in how to do this and how far to go with it. ShootSmallBall() could have been further broken down, calling another function to actually set the ball's properties, but that seemed excessive. It's pretty simple as it is.

In Waxface's case, I'm thinking something like this (very rough, I haven't looked at it extensively):

run():
    // Initialization and intro (intro might be another function)
    Repeat forever:
        Walk around
        Bounce around

WalkAround():
    Repeat forever:
        Until it's time to attack:
            Walk
            If on fire, return
        If health>=50% or randomly:
            Do fireball attack
        Else:
            Do slam attack

FireballAttack():
    // ... (return if on fire)

SlamAttack():
    // ... (return if on fire)

BounceAround():
    Pick an angle
    While still on fire:
        Move
        If bounced:
            Drop rock

BounceMove():
    // ... (return new angle and/or whether it bounced)

DropRock():
    // ...

That's omitting a lot of details, obviously, since you already know them and it's 1:45 AM. But you get the idea.

 

 

//I wonder if Saffith ever thought this would be used outside of Ghost's own internal functions?

I did not. Whatever works, I guess.


  • Evan20000 and Matthew like this

#6 Evan20000

Evan20000

    P͏҉ę͟w͜� ̢͝!

  • Members
  • Real Name:B̵̴̡̕a҉̵̷ņ̢͘͢͜n̷̷ę́͢d̢̨͟͞
  • Location:B̕҉̶͘͝a̶̵҉͝ǹ̵̛͘n̵e̸͜͜͢d҉̶

Posted 28 February 2020 - 04:29 AM

That's actually really insightful. I posted that one because it was ugly; I knew I needed to be able to be able to terminate various states/attacks in the event he gets lit on fire, but making each attack a boolean that returns a value if he ignites preemptively is something I hadn't considered and saves a lot of space. The main reason I haven't made this commonplace is that more complex bosses would tend to need to pass in a mountain of variables (Or even whole arrays in some cases) into each attack function and it'd look hideous. That said, you're absolutely right and I just need to find some organization system that would allow me to do this with some degree of sanity.

Spoiler


Edited by Evan20000, 28 February 2020 - 04:38 AM.


#7 Saffith

Saffith

    IPv7 user

  • ZC Developers

Posted 28 February 2020 - 10:24 AM

Yeah, one thing I really wish ZScript had is instance variables. If functions get too unwieldy, you can kind of fake it by putting them in an array and sticking it in this->Misc. Using them gets a bit uglier, though. Instead of, say,
this->Data=baseCombo;
you'd have something like
int vars=this->Misc[INSTANCE_VARS];
this->Data=vars[VAR_BASE_COMBO];
In some cases, you'd probably want more functions for clarification.
bool OnFire(ffc this) {
    return this->Misc[INSTANCE_VARS][VAR_ON_FIRE]!=0;
}
 
That's also an example of abstraction, by the way, though perhaps a bit borderline. Abstraction doesn't just mean simplifying complex operations; it means putting things in terms that are meaningful where they're used. And OnFire(this) is pretty meaningful.
  • Evan20000 likes this

#8 Evan20000

Evan20000

    P͏҉ę͟w͜� ̢͝!

  • Members
  • Real Name:B̵̴̡̕a҉̵̷ņ̢͘͢͜n̷̷ę́͢d̢̨͟͞
  • Location:B̕҉̶͘͝a̶̵҉͝ǹ̵̛͘n̵e̸͜͜͢d҉̶

Posted 28 February 2020 - 08:00 PM

Array stuffing is something I do from time to time as well, but usually for variables that might change and not as a glorified constant table such as combo declarations.

 

bool OnFire(ffc this) {
    return this->Misc[INSTANCE_VARS][VAR_ON_FIRE]!=0;
}

I wasn't even aware you could do that.
 

#9 Saffith

Saffith

    IPv7 user

  • ZC Developers

Posted 28 February 2020 - 08:24 PM

Oh... Actually, you can't, or at least not in 2.50. Should've tested it first. :P

 

But yeah, it's a good way to handle stuff like that. I used that approach extensively in the Mega Man engine as a simple inheritance mechanism. Every actor has its data stored in an array, and one element of that is an arbitrary number. Enemies and items use that for another array with their additional data.


  • Evan20000 likes this

#10 Evan20000

Evan20000

    P͏҉ę͟w͜� ̢͝!

  • Members
  • Real Name:B̵̴̡̕a҉̵̷ņ̢͘͢͜n̷̷ę́͢d̢̨͟͞
  • Location:B̕҉̶͘͝a̶̵҉͝ǹ̵̛͘n̵e̸͜͜͢d҉̶

Posted 28 February 2020 - 11:16 PM

I feel like I'm barely getting script-literate enough to parse how that engine works. When I'm done with my current thing, I might need to sit down and give that a look and see how it works (or at least make an attempt).



#11 Emily

Emily

    Scripter / Dev

  • ZC Developers

Posted 29 February 2020 - 12:56 AM

Oh... Actually, you can't, or at least not in 2.50. Should've tested it first.

ah, `arr[ind][ind]` should work fine in 2.55!



#12 Timelord

Timelord

    The Timelord

  • Banned
  • Location:Prydon Academy

Posted 02 March 2020 - 06:51 AM

Oh... Actually, you can't, or at least not in 2.50. Should've tested it first. :P
 
But yeah, it's a good way to handle stuff like that. I used that approach extensively in the Mega Man engine as a simple inheritance mechanism. Every actor has its data stored in an array, and one element of that is an arbitrary number. Enemies and items use that for another array with their additional data.

 
2.50.x/2.53.x is picky about this. It does work in 2.55, but in 2.53 the syntax requires declaring a second pointer
 
Spoiler

 
 
int ptr = arr[indx];
if ( ptr[indx2] )
 
We added a lot of array syntax, so we can also cover that here.
 
One thing I'll note is that the !=0 is meaningless in context, ad actually adds overhead as it forces a register compare.
 
2.55 Allows stuff such as:
 
int arr[2, 120]; //Creates array sized 240
arr[2,120] = 6; //Accesses index 121
 
May be imperfect, but array[][] is legal now to access array indices of pointers stored in an int as an array.
 
This will likely have further revisions for true 2D/3D arrays and structs in the future, when we have real pointer/address stuff.
 
I've been writing a final set of docs (ZScript Manual) and a style guide. I'll post them when done.
 
@Saffith: We're getting closer to Linux builds for you.
 
Re: Abstraction
IMO, abstraction should be minimised to where it is beneficial. Nothing is more annoying hen referencing one function after another (possible in multiple files), just o determine what something does.
 
Another useful thing:
 
I 2.50.x, 2.53, and 2.55, you have options on creating functions that can be accessed from any scope, but aren't cluttering he global namespace. This also applies to global vars and arrys, although in versions < 2.54, these cannot begin initialised.
 
In 2.50.x2.53.x, if you want a function that is available at all scopes n('global'), but ou don't want to pollute the global namespace, you can make a generic script that acts like a namespace:
 
 
ffc script items
{
    void run(){}
    void collision(item x)
    {
        int ax = Link->X + Link->HitXOffset;
        int bx = b->X + b->HitXOffset;
        int ay = Link->Y + Link->HitYOffset;
        int by = b->Y + b->HitYOffset;
        return RectCollision(ax, ay, ax+Link->HitWidth, ay+Link->HitHeight, bx, by, bx+b->HitWidth, by+b->HitHeight) && (Link->Z + Link->HitZHeight >= b->Z) && (Link->Z <= b->Z + b->HitZHeight);
    }
}
 
You can call this globally as items.collision(itm);
 
Doing this prevents needing a lot of longworded function identifiers, by using the same identifier, per namespace.
 
In 2.55, you can use real namespaces:
 
 
namespace items
{
    void collision(item x)
    {
        int ax = Link->X + Link->HitXOffset;
        int bx = b->X + b->HitXOffset;
        int ay = Link->Y + Link->HitYOffset;
        int by = b->Y + b->HitYOffset;
        return RectCollision(ax, ay, ax+Link->HitWidth, ay+Link->HitHeight, bx, by, bx+b->HitWidth, by+b->HitHeight) && (Link->Z + Link->HitZHeight >= b->Z) && (Link->Z <= b->Z + b->HitZHeight);
    }
}
 
The call here is (your option) items::collision(itm) or items..collision(itm) .
 
Namespace resolution tokens are :: and ..
 
You can select whichever you prefer. 2.55 also allows declaring untyped scripts if you prefer that format of dereference:
 
 
untyped script items
{
    void collision(item x)
    {
        int ax = Link->X + Link->HitXOffset;
        int bx = b->X + b->HitXOffset;
        int ay = Link->Y + Link->HitYOffset;
        int by = b->Y + b->HitYOffset;
        return RectCollision(ax, ay, ax+Link->HitWidth, ay+Link->HitHeight, bx, by, bx+b->HitWidth, by+b->HitHeight) && (Link->Z + Link->HitZHeight >= b->Z) && (Link->Z <= b->Z + b->HitZHeight);
    }
}
 
Here, you need no run() function, and you can still call items.collision(itm) as with any other script.
 
For years, I have been using this technique to give me identifiers that act as type.action() , ad this both prevents polluting the global namespace with stuff like ItemCollision(), and thus header conflicts, bt it is also immensely readable.
 
The namespace identifier tells you what kind of type the function affects, ad the function name details what i does.
 
Anyway, overly abstracting your code can lead to headaches. If you call a number of instrructions repeatedly, then it should become a function, but if you have a set of calls in only one place, don't abstract them into functions just becaue you can. First consider why you want to consider doing that ad if you lack a sufficient reason, then i likely doesn't need abstraction.
 
I often run across C++, Java, C#, ad Lua stuff that is abstracted for its own sake, and this creates a maze of code in which you have no clue what any one function does without checking what each of its calls does.
 
Anyway, the docs will be done 'soon', ad they cover a comprehensive basis for all syntax. I rebased them on a combination of the GNU C docs, and my own docs. I think that reading these will be a good primer in using he language.
 
@Saffith: Why do you you still use EOL braces?
While it is a valid C style, it is not something that IMO we should use as a guide in examples, for two reasons:
 
1. It is easy to mismatch or misplace closing braces.
2. Code editors won't line-match those braces evenly.
 
FWIW, this is maddening while I work on Ghos,zh and other headers,
 
take this snippet as an example:
 
 
if ( Collision(a) && Link->X > 16 && Link->Y > 16 && Link->X < 200 && Link->Y < 180 && Link->InputA && (Link->InputUp || Link->InputDown) && OnSizeViewPlatform() ){
{	
	DoAThing();
}
 
If you forget that you have an EOL brave, it is somewhat troubling to see an error pop up here, as the EOL brace will be out of view in your editor. This is especially true in complex, nested blocks.
 
While this is purely personal, the industry has moved almost entirely to placing braces o their own lines, except for short exprs where the braces and the expr fir all on one line (compactness).
 
My style guide covers this (actually both, with pros ad Cons for each), bu I always advise placing braces on NL.
 
We also need to cover code editors. People should avoid using Windows Notepad for ZScript, abd we have options available for Windows and for Linux, that are superb. The issue to discuss are the Pros/Cons of using dedicated code editors or extended text editors.
 
We have language defs with full syntax highlighting for SciTE ad for Notepad++, as well as download packages for them on zc.com.

#13 Avaro

Avaro

    o_o

  • Members
  • Real Name:Robin
  • Location:Germany

Posted 02 March 2020 - 08:44 AM

Arr[indx[subindx]] has always been valid. I think end of line braces look nicer because it saves a lot of space and there are no problems. And yes, I highly suggest coding with code editors, as they greatly improve your convenience for coding.

 

Soooo, I suppose I can post a script for critique too. I'm not particularly looking for any, but why not? I know that there's a bit of repeated code there which could have probably been avoided with some more effort. (This is a scripted trap. Simple. Free for use.)

// The FFC is placed on the screen and given a combo and speed so it starts moving.
// It will then automatically bounce off walls.
// D0: 0 = back and forth or diagonal, -1 = turn countclockwise after hitting wall, 1 = turn clockwise after hitting wall
// D1: Delay before moving again after hitting wall.
ffc script Trap{
	void run(int type, int delay){
		this->Flags[FFCF_ETHEREAL] = true;
		eweapon wpn = CreateEWeaponAt(EW_SCRIPT10, this->X, this->Y);
		wpn->Damage = 4;
		wpn->DrawXOffset = -1000;
		while(true){
			// check if walls are ahead
			if ( this->Vy > 0 && !CanWalk(this->X, this->Y, DIR_DOWN, 2, true) ) {
				this->Y = ComboY(ComboAt(this->X+8, this->Y+8)); // snap to Y grid
				int oldv = this->Vy;
				this->Vy = 0;
				Game->PlaySound(57);
				Waitframes(delay);
				if ( type == 0 )
					this->Vy = oldv * -1;
				else
					this->Vx = oldv * type * -1;
			}
			if ( this->Vx < 0 && !CanWalk(this->X, this->Y, DIR_LEFT, 2, true) ) {
				this->X = ComboX(ComboAt(this->X+8, this->Y+8)); // snap to X grid
				int oldv = this->Vx;
				this->Vx = 0;
				Game->PlaySound(57);
				Waitframes(delay);
				if ( type == 0 )
					this->Vx = oldv * -1;
				else
					this->Vy = oldv * type;
			}
			if ( this->Vy < 0 && !CanWalk(this->X, this->Y, DIR_UP, 2, true) ) {
				this->Y = ComboY(ComboAt(this->X+8, this->Y+8)); // snap to Y grid
				int oldv = this->Vy;
				this->Vy = 0;
				Game->PlaySound(57);
				Waitframes(delay);
				if ( type == 0 )
					this->Vy = oldv * -1;
				else
					this->Vx = oldv * type * -1;
			}
			if ( this->Vx > 0 && !CanWalk(this->X, this->Y, DIR_RIGHT, 2, true) ) {
				this->X = ComboX(ComboAt(this->X+8, this->Y+8)); // snap to X grid
				int oldv = this->Vx;
				this->Vx = 0;
				Game->PlaySound(57);
				Waitframes(delay);
				if ( type == 0 )
					this->Vx = oldv * -1;
				else
					this->Vy = oldv * type;
			}
			
			// handle the weapon
			if ( !wpn->isValid() ) {
				wpn = CreateEWeaponAt(EW_SCRIPT10, this->X, this->Y);
				wpn->Damage = 4;
				wpn->DrawXOffset = -1000;
			}
			
			wpn->DeadState = WDS_ALIVE;
			wpn->X = this->X;
			wpn->Y = this->Y;
			
			if ( this->Vx > 0 )
				wpn->Dir = DIR_RIGHT;
			else if ( this->Vx < 0 )
				wpn->Dir = DIR_LEFT;
			else if ( this->Vy > 0 )
				wpn->Dir = DIR_DOWN;
			else if ( this->Vy < 0 )
				wpn->Dir = DIR_UP;
			Waitframe();
		}
	}
}


#14 Emily

Emily

    Scripter / Dev

  • ZC Developers

Posted 02 March 2020 - 12:08 PM

arr[indx[subinx]] isn't what we were talking about; we were talking about arr[indx][subindx]



#15 Saffith

Saffith

    IPv7 user

  • ZC Developers

Posted 02 March 2020 - 04:28 PM

One thing I'll note is that the !=0 is meaningless in context, ad actually adds overhead as it forces a register compare.

That would be pretty easy to optimize out, wouldn't it? It's not a big deal either way, but I typically don't like implicitly casting numbers to bools.
 

Anyway, overly abstracting your code can lead to headaches. If you call a number of instrructions repeatedly, then it should become a function, but if you have a set of calls in only one place, don't abstract them into functions just becaue you can. First consider why you want to consider doing that ad if you lack a sufficient reason, then i likely doesn't need abstraction.

It can be overdone, sure. Too many functions doing too little, it can get hard to find code that actually does anything. It takes some effort, though. I think it's generally better to err on the side of too much abstraction at first. It's easier to remove excessive function calls than to find where you should have put them in the first place, and it's easier to write code without thinking about high-level behavior and low-level implementation details both at once.

 

The limitations of the language are an issue here, too. Most languages have instance variables. In ZScript, functions are usually the closest you can get in terms of readability, though using ffc->Misc or another array with reasonable constant indices is often good enough.
 

@Saffith: Why do you you still use EOL braces?

Not "still"; I switched to them about a year ago, when I learned Rust. The extra vertical whitespace came to bother me more over time, and it's particularly bad in Rust, where short, inline blocks are common and braces after if and else can never be omitted.
 

If you forget that you have an EOL brave, it is somewhat troubling to see an error pop up here, as the EOL brace will be out of view in your editor. This is especially true in complex, nested blocks.

I would say that's a different problem. If you've got lines so long you can't see the ends of them, they're too long. And long, complex if statements are especially bad.
 

While this is purely personal, the industry has moved almost entirely to placing braces o their own lines, except for short exprs where the braces and the expr fir all on one line (compactness).

You're looking at a different industry than I am. Rust, Dart, Swift, and Reason all have official formatting tools that default to end-of-line braces. Go actually requires them; putting an opening brace on its own line is a syntax error. Google, Oracle, Mozilla, and Linux all recommend or require EOL braces in their style guides. WebKit and Microsoft go the other way. There's not a clear consensus, but EOL braces are more common in the standards I'm aware of.
 

Soooo, I suppose I can post a script for critique too. I'm not particularly looking for any, but why not? I know that there's a bit of repeated code there which could have probably been avoided with some more effort.

Yeah, the repeated bits could be worked into a separate function. But I don't think they should be, or at least not just one function.

I see a couple of issue with traps moving diagonally. Say a trap is moving up-left and hits a wall to the left. It sees that it can't keep going left, then stops for a bit and bounces... But it only stops moving horizontally. It keeps moving vertically, and the script is doing nothing during that time. The eweapon becomes unaligned and isn't recreated if destroyed. If there's another wall above the trap, it will keep moving right into it.

The simplest thing to do would be to stop vertical and horizontal movement both at once, and that might be good enough for your purposes. But there's still a minor issue in that case: if the trap hits a corner directly - if it can't continue vertically or horizontally - it will stop twice as long and play the bounce sound twice.

The logic, as you would understand it intuitively, is something like this:

If the trap can't keep moving:
    Stop for a moment
    Change direction

But if you think about it carefully, there are a few different cases that have slightly different behaviors. The trap may be blocked vertically, horizontally, or both at once, and you need to take all three into account.

When you have multiple, overlapping causes and effects, you have to be sure to consider possible combinations. You have basically two options: either add more and more complex if checks so that only one can trigger; or check all the possible causes, condense the results, and then decide on effects. Option two may still involve a bit of option one when the effects have both common and different elements, as in this case. You could do something like this, for instance:

hitH=!CanMoveH(this);
hitV=!CanMoveV(this);
if(hitH || hitV) {
    float oldVx=this->Vx;
    float oldVy=this->Vy;
    this->Vx=0;
    this->Vy=0;
    Game->PlaySound(57);
    Waitframes(delay);
    
    // Change direction
    if(hitH && hitV) {
        // ...
    }
    else if(hitH) {
        // ...
    }
    else { // hitV
        // ...
    }
}

Handling turning and bouncing logic together might get more complicated. I'd probably give cycling and bouncing traps separate main loops, or even make them separate scripts.

Is there a particular reason for using an eweapon instead of a damage combo? Since you're setting its Dir, I'm guessing it's for directional knockback, or maybe shield interaction. If you need it, you need it, but I wouldn't recommend it if you don't. It makes for more complex code and therefore greater potential for bugs. If you do need it, you might want to write a custom Waitframe function to ensure it's always handled properly. It'd either need to take the current weapon pointer as an argument and return an updated one, or you could put the pointer in an array and pass that in.


  • Avaro likes this


0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users