[JASS] Prime number generator is it good?

Status
Not open for further replies.
i just made this trigger in JassCraft, its my first scratch jass trigger, it may be crappy, but meh, i wanted to learn from my mistakes, considering this is my first solo trigger. Thanks.
JASS:
global I Primenumber
function Trig_NewTrigger2_Conditions takes nothing returns boolean
    
    return 
endfunction

function Trig_NewTrigger2_Actions takes integer I returns boolean B
local integer Integer_A
  loop
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
     if DevideNumbers(Integer_A) == null
     Then return false
     else
  endloop
endfunction

//==== Init Trigger NewTrigger ====
function InitTrig_NewTrigger2 takes nothing returns nothing
    set gg_trg_NewTrigger2 = CreateTrigger()
    //call TriggerRegister__(gg_trg_NewTrigger, )
    call TriggerAddCondition(gg_trg_NewTrigger2, Condition(function Trig_NewTrigger2_Conditions))
    call TriggerAddAction(gg_trg_NewTrigger2, function Trig_NewTrigger2_Actions)
endfunction    
function Trig_NewTrigger_Conditions takes boolean returns integer
local integer Integer_A
  loop
     if  function Trig_NewTrigger2_Actions = true
     then set udg_Prime_Number = Integer_A 
     else
  endloop
endfunction

function Trig_NewTrigger_Actions takes nothing returns nothing
    
endfunction

//==== Init Trigger NewTrigger ====
function InitTrig_NewTrigger takes nothing returns nothing
    set gg_trg_NewTrigger = CreateTrigger()
    //call TriggerRegister__(gg_trg_NewTrigger, )
    call TriggerAddCondition(gg_trg_NewTrigger, Condition(function Trig_NewTrigger_Conditions))
    call TriggerAddAction(gg_trg_NewTrigger, function Trig_NewTrigger_Actions)
endfunction
JASS:
function Trig_NewTrigger_Conditions takes boolean returns integer
local integer Integer_A
local boolean Boolean_A
  loop
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
     if DevideNumbers(Integer_A) == null
     then set udg_Prime_Number = Integer_A 
     else
     endif
  endloop
endfunction

function Trig_NewTrigger_Actions takes nothing returns nothing
    
endfunction

//==== Init Trigger NewTrigger ====
function InitTrig_NewTrigger takes nothing returns nothing
    set gg_trg_NewTrigger = CreateTrigger()
    //call TriggerRegister__(gg_trg_NewTrigger, )
    call TriggerAddCondition(gg_trg_NewTrigger, Condition(function Trig_NewTrigger_Conditions))
    call TriggerAddAction(gg_trg_NewTrigger, function Trig_NewTrigger_Actions)
endfunction
(2nd thing, this is different then the first one)

and please dont be like "That leaks, idiot!" or flaming stuff, considering this is my first go at jass, well, i think its good :D
And i didn't know if i should of set it to null at the end of the trigger, because it could of not worked properly then.

This is the best fourm i found for posting this in, if there's another one, just move it.
 
Level 9
Joined
Nov 28, 2008
Messages
704
If this is a prime number generator, you could have just used a vJass scope or library and not played with triggers, not to mention if your using JASS you should be using JNGP, therefore you should be using globals made in JASS instead of the udg_ ones made in the variable editor.

If it's your first one, good job getting into JAS, but you could probably do a bit better.

In the second trigger you also have an infinite loop. Is this intentional? You need an exitwhen in every loop, unless you plan on having a game long thing running.

Your syntax could be improved upon..

JASS:
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
if DevideNumbers(Integer_A) == null
     then set udg_Prime_Number = Integer_A
     else
     endif

Should be indented just a bit to increase readability...
 
vJass is the same as Jass, just with more awsomeness (OOP, scopes, libraries, freedom of global declaration...)

Jass is CaSE SENSitIve, so you can't use "Then" as a substitute for "then", or anything similar. Also, you can't compare functions to booleans:

JASS:
if function Trig_NewTrigger2_Actions = true
then set udg_Prime_Number = Integer_A
else

The "then" has to be on the same line as the if statement as well. And seriously, what is the point in making empty triggers?

I suggest you convert a few GUI triggers to Jass, and see if you can learn from that.
 
Level 24
Joined
May 9, 2007
Messages
3,563
JASS:
function Trig_NewTrigger2_Conditions takes nothing returns boolean

    return
endfunction

This function DOESN'T DO ANYTHING.

JASS:
function Trig_NewTrigger2_Actions takes integer I returns boolean B
local integer Integer_A
  loop
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
     if DevideNumbers(Integer_A) == null
Then return false
     else
  endloop
endfunction


This loop will never exit. also your whole if/then/else structure is screwed up.

JASS:
function Trig_NewTrigger_Actions takes nothing returns nothing

endfunction

Totally empty function. Wtf.

JASS:
function Trig_NewTrigger_Conditions takes boolean returns integer
local integer Integer_A
local boolean Boolean_A
  loop
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
if DevideNumbers(Integer_A) == null
     then set udg_Prime_Number = Integer_A
     else
     endif
  endloop
endfunction

If the iteration count of the loop, when put into a miss-spelled function that doesn't exist comes out to equal null then set a variable keep looping forever. WHAT. THE. HELL?

JASS:
function Trig_NewTrigger_Actions takes nothing returns nothing

endfunction

And another totally empty function.

I'm not going to give any methodology suggestions, because I can't work out wtf you are trying to do.
I am also not going to look at your whole Acions/Events/Conditions thing because your names are all retarded, and hence un-readable.
 
Last edited:
Level 2
Joined
Jul 17, 2009
Messages
27
Firstly, I'd recommend going straight to some basic aspects of vJASS, in order to make things easier for both yourself, and people who need to decipher your triggers.

So, let's get started!


First things first, we're going to put your trigger inside a 'scope'. A scope is basically a container that keeps your trigger in and other triggers out.

JASS:
scope NewTrigger

Note that I've named the scope "NewTrigger", which is the name of the test trigger you already have.


Next up, we have globals. You've written out your global decleration like so:
JASS:
global I Primenumber

It should be:
JASS:
globals
 private integer PrimeNumber = 1
 // Here we declare the variable PrimeNumber to
 // be an integer, and start it at 1.
endglobals


This function is blank. We can remove it (it serves no purpose). It also has a syntax error (you haven't told it <what> to return). Syntax is important - there is a specific way to write in JASS and if you don't follow this the code will not compile. In can also crash the WE's original compiler.
JASS:
function Trig_NewTrigger2_Conditions takes nothing returns boolean
    
    return 
endfunction


There are several problems with this function:
JASS:
function Trig_NewTrigger2_Actions takes integer I returns boolean B
local integer Integer_A
  loop
     set Integer_A = (Integer_A + 1)
     TriggerSleepAction(0.01)
     if DevideNumbers(Integer_A) == null
     Then return false
     else
  endloop
endfunction

First up, it takes "integer I", which was the global you had originally tried to use. Do not mix up your variables! You should make sure every variable is unique.

Secondly, we can change the name of this trigger. With scopes, we can call it anything we like as long as we append "private" to it, telling the compiler that nothing outside the scope (this trigger) may access it.

Finally, that loop will never end. This is bad! Only ever create infinite loops if you know exactly what you're doing. Have a look at this redone function:
JASS:
private function IsPrimeNumber takes integer n returns boolean
// Our function declaration. This function is called
// "IsPrimeNumber", and when we call this function,
// we will give it an integer. The function must give
// us a boolean (true/false) value as a result.
local integer c = 2
// We will use integer "c" to check if integer "n"
// is a prime number. Integer "n" is the integer
// that is given to this function when it is called.
 loop
  exitwhen c > n/2
  // This tells the loop when to finish. I've placed this at the
  // beginning of the loop to make it easier to read - we
  // know exactly when the loop will end. I am assuming
  // you understand enough number theory to realise
  // what I'm doing.
  if ModuloInteger(n, c) == 0 then
  // An if statement must be written in the above manner.
  // The parts between "if" and "then" must return a boolean.
   return false
   // Here we are returning false. This will exit the loop and end
   // the function. We do this once we discover that n is not
   // a prime number.
  endif
  set c = c + 1
 endloop
return true
// Since "n" has not been shown to NOT be a prime number,
// we now assume that it is.
endfunction

Looking at the next functions you have:
JASS:
//==== Init Trigger NewTrigger ====
function InitTrig_NewTrigger2 takes nothing returns nothing
    set gg_trg_NewTrigger2 = CreateTrigger()
    //call TriggerRegister__(gg_trg_NewTrigger, )
    call TriggerAddCondition(gg_trg_NewTrigger2, Condition(function Trig_NewTrigger2_Conditions))
    call TriggerAddAction(gg_trg_NewTrigger2, function Trig_NewTrigger2_Actions)
endfunction    
function Trig_NewTrigger_Conditions takes boolean returns integer
local integer Integer_A
  loop
     if  function Trig_NewTrigger2_Actions = true
     then set udg_Prime_Number = Integer_A 
     else
  endloop
endfunction

function Trig_NewTrigger_Actions takes nothing returns nothing
    
endfunction

You've sort of got things mucked about here, so we're going to scrap it all and start fresh.

What we're going to do is create a trigger that, every 2 seconds, displays the next prime number. We'll use the previous function IsPrimeNumber to do that. Now, when we create a basic trigger, we give it two or more parts: an initialisation function, which is run when the map starts and creates our trigger. An actions function, which is what the trigger runs whenever its event occurs. We can also add conditions functions, which allow us to make sure the event we want is the correct one.

For this trigger, we will need the actions and initialisation. We will write the actions first, because the initialisation function needs to have the action function 'declared' before it can use it to create a trigger.

Here we go:

JASS:
private function Actions takes nothing returns nothing
// We declare the function here. This function does not take
// any arguments, and does not give us any results. This
// is normal for action functions.
local boolean finish = false
// We will use this to check whether to exit our loop.
// Because this is local, it will reset every time the trigger
// runs.
loop
 exitwhen finish
 // finish is a boolean, so we don't need to compare it
 // to anything.
 if IsPrimeNumber(PrimeNumber) then
 // Because IsPrimeNumber returns a boolean, we don't
 // need to compare it to anything in order to use it in
 // an if statement.
  call BJDebugMsg(I2S(PrimeNumber)+" is a prime number!")
  set finish = true
  // Once we find a prime number, we set finish to true so that
  // the loop will end. This whole function will run again in 2
  // seconds, because of the period trigger it is a part of.
 endif
 set PrimeNumber = PrimeNumber + 1
 // We increase PrimeNumber so we can check if the
 // next number is prime. Unlike "finish", PrimeNumber
 // is a global and will be remembered between events.
endloop
endfunction


Now, we need to initialise our trigger. Because we are using a scope, we can optionally declare any function to be our initialiser. However, since we haven't done so, we will need to use the default initialiser, InitTrig. This must be declared public for it to work.

JASS:
public function InitTrig takes nothing returns nothing
 local trigger t = CreateTrigger()
 // We create a trigger, and assign the variable t to it.
 call TriggerRegisterTimerEvent(t, 2.0, true)
 // We add the event to the trigger. This trigger will run every
 // two seconds.
 call TriggerAddAction(t, function Actions)
 // And this is the function that is called when the trigger runs.
 set t = null
 // This isn't strictly necessary, since we never destroy the
 // trigger, but is good practice. This has to do with memory
 // management.
endfunction

Finally, we have to end the scope.

JASS:
endscope


Hope that helps!


Edit: Final code for this trigger:

JASS:
scope NewTrigger

globals
 private integer PrimeNumber = 1
 // Here we declare the variable PrimeNumber to
 // be an integer, and start it at 1.
endglobals

private function IsPrimeNumber takes integer n returns boolean
// Our function declaration. This function is called
// "IsPrimeNumber", and when we call this function,
// we will give it an integer. The function must give
// us a boolean (true/false) value as a result.
local integer c = 2
// We will use integer "c" to check if integer "n"
// is a prime number. Integer "n" is the integer
// that is given to this function when it is called.
 loop
  exitwhen c > n/2
  // This tells the loop when to finish. I've placed this at the
  // beginning of the loop to make it easier to read - we
  // know exactly when the loop will end. I am assuming
  // you understand enough number theory to realise
  // what I'm doing.
  if ModuloInteger(n, c) == 0 then
  // An if statement must be written in the above manner.
  // The parts between "if" and "then" must return a boolean.
   return false
   // Here we are returning false. This will exit the loop and end
   // the function. We do this once we discover that n is not
   // a prime number.
  endif
  set c = c + 1
 endloop
return true
// Since "n" has not been shown to NOT be a prime number,
// we now assume that it is.
endfunction

private function Actions takes nothing returns nothing
// We declare the function here. This function does not take
// any arguments, and does not give us any results. This
// is normal for action functions.
local boolean finish = false
// We will use this to check whether to exit our loop.
// Because this is local, it will reset every time the trigger
// runs.
loop
 exitwhen finish
 // finish is a boolean, so we don't need to compare it
 // to anything.
 if IsPrimeNumber(PrimeNumber) then
 // Because IsPrimeNumber returns a boolean, we don't
 // need to compare it to anything in order to use it in
 // an if statement.
  call BJDebugMsg(I2S(PrimeNumber)+" is a prime number!")
  set finish = true
  // Once we find a prime number, we set finish to true so that
  // the loop will end. This whole function will run again in 2
  // seconds, because of the period trigger it is a part of.
 endif
 set PrimeNumber = PrimeNumber + 1
 // We increase PrimeNumber so we can check if the
 // next number is prime. Unlike "finish", PrimeNumber
 // is a global and will be remembered between events.
endloop
endfunction

public function InitTrig takes nothing returns nothing
 local trigger t = CreateTrigger()
 // We create a trigger, and assign the variable t to it.
 call TriggerRegisterTimerEvent(t, 2.0, true)
 // We add the event to the trigger. This trigger will run every
 // two seconds.
 call TriggerAddAction(t, function Actions)
 // And this is the function that is called when the trigger runs.
 set t = null
 // This isn't strictly necessary, since we never destroy the
 // trigger, but is good practice. This has to do with memory
 // management.
endfunction
endscope
 
Last edited:
Level 14
Joined
Nov 20, 2005
Messages
1,156
To sum up:

You need to learn Jass. Right now, you don't know Jass. At all. I'm going to be honest with you, this is probably the worst code *I've* ever seen, and that means it's really, really bad. So go back to basics. You need to understand what you're doing, read a few basic Jass tutorials, etc.
 
Status
Not open for further replies.
Top