Skip to content
This repository has been archived by the owner on Sep 2, 2019. It is now read-only.

Attach Added #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attach Added #14

wants to merge 1 commit into from

Conversation

LinearBoundedAutomaton
Copy link

Adds functionality for an attach command

Adds functionality for an attach command
Copy link
Owner

@ElusiveMori ElusiveMori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few common conventions used in Wurst (and in YARP2) that are missing. There's a few written down in Wurst's manual - https://wurstlang.org/manual.html#coding-conventions

Overall, I highly recommend you to skim over the manual in whole, so that you can get a better grasp on the language. It's very different from both JASS and vJASS and the paradigms are not at all the same.

Prefer to use existing Wurst functionality over writing everything from scratch like you would in vJASS, it's really much easier.

There's a lot of things I skimmed over, as well, but here are some general points:

  • Prefer to use var/let notation over explicit type annotations.
  • Prefer to use vec2 and vec3 tuples for storing positional data vs storing each component individually. Not only is it more compact, but there's a whole lot of extension functions and utilities already built-in.
  • Try to avoid using JASS natives directly. Most natives already have a Wurst wrapper around them. If you're not sure, you can do a file search in the stdlib directory for the native name, and find which wrappers there are for it.

I'll do a more thorough review when the points above are implemented. Sorry for so many change requests, but it's very important to me that the code is both consistent in style and is idiomatic. Thanks for participating!

let b = arguments.getReal(2)
let c = arguments.getReal(3)
let d = arguments.getReal(4)
let e = arguments.getBool(5)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use better variable names.




function g_Destroy()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this? Why not ondestroy?

AttachLink is the link which holds the host and guest aUnits as well as their metadata

*/
public class aUnit
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use CapitalCamelCase for class names.

@@ -0,0 +1,72 @@
package aUnit
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use CapitalCamelCase for package names.

static HashSet<AttachLink> ConnectionStorage
HashSet<AttachLink> guestConnections
//run in miscinit
static function init_Connection_Storage(timer clock, real timeout)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use lowerCamelCase for functions/methods.

TimerStart(clock, timeout, true, function updateStoredConnections)

static function updateStoredConnections()
HLIterator<AttachLink> iterator = ConnectionStorage.iterator()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use let iterator = ... vs explicit type annotations.

xNew = x + Cos(currentLink.angle * DEGTORAD + f * DEGTORAD) * currentLink.distance
yNew = y + Sin(currentLink.angle * DEGTORAD + f * DEGTORAD) * currentLink.distance
zNew = z
SetUnitX(guest, xNew)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use unit.setXY(vec2) vs natives.

real yNew
real zNew
if(currentLink.staticAngle)
xNew = x + Cos(currentLink.angle * DEGTORAD) * currentLink.distance
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the angle tuple for representing angles vs manual degrees/radians conversions. More can be found in stdlib's Angle.wurst

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants