I'm putting off doing other work so have a little bit of a code review. I take it you're new to CON. From your other comments it seems like you're pulling a lot of this from other mods. Hopefully this will help you get a better understanding of the language.
gamevar ACCESSKEY2 0 1
gamevar CUSTOMKEYCARDPOS 0 0
gamevar KEYCARD_CURRENT_PAL 0 0
gamevar CKEYCARDPAL 0 0
gamevar CKEYCARDTILE 0 0
gamevar KEYCARD_TEMP 0 2
gamevar KEYCARD_TEMP2 0 2
gamevar KEYCARD_TEMP3 0 2
gamevar KEYCARD_TEMP4 0 2
gamevar KEYCARD_TEMP7 0 0
You can shorthand "gamevar" simply with "var". In fact there's a lot of shorthand stuff I'll be mentioning that makes reading code a lot easier.
For global gamevars (that is, those with the flag "0"), you don't actually have to declare the flag as gamevars default as global. Personally I don't use per-player gamevars either, unless you're writing a multiplayer mod then there's functionally no difference between global and per-player.
eventloadactor ACCESSCARD2
setactor[THISACTOR].xrepeat 28
setactor[THISACTOR].yrepeat 28
enda
You should use the EVENT_LOADACTOR event rather than eventloadactor, as eventloadactor and similar legacy EDuke functionality has been superceded by game events.
It's good practice to use curly braces to contain state, event, etc. code.
You can shorthand your setactor calls to "seta", and if you are working with THISACTOR then the spriteid brace can be omitted as well. So your code becomes:
onevent EVENT_LOADACTOR
{
seta .xrepeat 28
seta .yrepeat 28
}
endevent
ifspritepal 12
{
ifvarand ACCESSKEY2 1 { break }
else
{
addvar ACCESSKEY2 1
setvar KEYCARD_TEMP7 30
globalsound DUKE_GET
quote 43
palfrom 16 0 32
killit
break
}
}
ifspritepal 11
{
ifvarand ACCESSKEY2 2 { break }
else
{
[. . .]
}
}
[. . .]
The curly braces with the "break" in them should be replaced by "nullop" which does what you are intending on doing here. In fact, using curly braces on a single command is a bad practice and can slow down CON performance and should be avoided.
You should use an "else" statement between these ifspritepal calls. This would make them a single conditional chain, rather than each block being evaluated. As it is, the script checks against each conditional even if it finds a successful branch. Technically this is resolved by your use of "break", but only because there is no other code after the ifspritepal block. If there was code after then that you needed to execute, you wouldn't be able to use break here in that way.
Like shorthanding "setactor" to "seta", any gamevar if conditions don't require the implicit usage of "var" or "varvar" in them. So "ifvarand" (or "ifvarvarand") just becomes "ifand", and "addvar" just becomes "add".
You also reuse a lot of code in this section, which you could put into a separate state and save code size and make things a lot more readable.
With these changes:
defstate getcard
{
set KEYCARD_TEMP7 30
globalsound DUKE_GET
quote 43
palfrom 16 0 32
killit
}
ends
[. . .]
ifspritepal 12
{
ifand ACCESSKEY2 1
nullop
else
{
add ACCESSKEY2 1
state getcard
}
}
else
ifspritepal 11
{
ifand ACCESSKEY2 2
nullop
else
{
add ACCESSKEY2 2
state getcard
}
}
[. . .]
Moving on...
eventloadactor ACCESSSWITCH3
{
getactor[THISACTOR].pal KEYCARD_TEMP
setactor[RETURN].pal KEYCARD_TEMP
getactor[THISACTOR].hitag HITAGSAVED
getactor[THISACTOR].lotag LOTAGSAVED
setactor[THISACTOR].hitag 0
setactor[THISACTOR].lotag 0
setactor[THISACTOR].cstat 17
setactor[THISACTOR].clipdist 64
}
enda
I don't understand what the "setactor[RETURN]" line is supposed to be doing here. I don't think it accomplishes anything, eventloadactor doesn't use RETURN. I think you meant to put THISACTOR here. Otherwise the same changes apply as mentioned in the earlier use of eventloadactor, and this block can be streamlined.
state checkcounterpart2
setvar KEYCARD_TEMP 0
whilevarn KEYCARD_TEMP 16384
{
getactor[KEYCARD_TEMP].statnum KEYCARD_TEMP4
ifvarn KEYCARD_TEMP4 1024
{
getactor[KEYCARD_TEMP].picnum KEYCARD_TEMP2
ifvare KEYCARD_TEMP2 ACCESSSWITCH3
{
getactorvar[KEYCARD_TEMP].LOTAGSAVED KEYCARD_TEMP3
ifvarvare LOTAGSAVED KEYCARD_TEMP3 { setactor[KEYCARD_TEMP].picnum 15177 }
}
}
addvar KEYCARD_TEMP 1
}
ends
This code really sucks, but I don't blame you, because why it sucks is not immediately clear or well documented.
You should use "defstate" when defining new state blocks, and reserve using "state" when calling them from other locations in code.
The main thing is that while loops are really, really bad performance-wise. The exact reasons are technical in a way that I won't go into here, but whenever possible you should use a for loop instead of a while loop. For loops are, like, logarithmically more performant than while loops in CON. The while loop you're using here seems like it could even be replaced with a "for allsprites" loop, since it seems to be looking for any sprite with a matching LOTAGSAVED gamevar value. Also, when using for loops, it's not necessary to initialize the iterator gamevar to 0 as that's implied.
You can also directly compare sprite structs without assigning them to gamevars.
After changes:
defstate checkcounterpart2
{
for KEYCARD_TEMP allsprites
{
ifn sprite[KEYCARD_TEMP].statnum 1024
ife sprite[KEYCARD_TEMP].picnum ACCESSSWITCH3
ife actorvar[KEYCARD_TEMP].LOTAGSAVED LOTAGSAVED
seta[KEYCARD_TEMP].picnum 15177
}
}
ends
This code still kinda sucks but for different reasons. Checking the statnum for 1024 (invalid sprites) probably isn't necessary. You could probably also replace the allsprites loop (which is generally slow) with a more precisite "spritesofstatus", or shorthanded to "sprofstat" to look for just actors, assuming ACCESSSWITCH3 will always be an actor. Lastly, using magic numbers is always bad, so you should define 15177 somewhere and use that definition instead.
The only other real thing is down in the EVENT_DISPLAYWEAPON code, your switch case has "case 0" but you should change this to "default" to catch unexpected values.
The rest of the code seems to fall into a lot of these traps, especially lots of recycled code that could be a separate state block, and tons of curly braced single line commands. You do that a lot and it's pretty bad.
Happy coding.
Help
Duke4.net
DNF #1
Duke 3D #1
Reaper_Man, on 03 November 2024 - 09:48 AM, said:
