Feature/rewrite #2
Conversation
|
|
||
| public override void Load() | ||
| { | ||
| Instance = this; |
There was a problem hiding this comment.
Looks fine. Make sure to set Instance = null in Unload()
(static variables otherwise linger when people unload the mod)
Do this for any static variables (recommended: Unload() method in places where statics remain)
| string prefixes = ""; | ||
| string suffixes = ""; | ||
| try | ||
| PrefixNPC prefixNPC = GetGlobalNPC<PrefixNPC>(); |
There was a problem hiding this comment.
As discussed IRL, can benefit from OO-approach (NetHandler / NetPacket class etc.)
Might do it myself
| { | ||
| public class CryophobicPrefix : NPCPrefix | ||
| { | ||
| public override string Type => DebuffGroup.NAME; |
There was a problem hiding this comment.
To align better with vanilla/tml design, type is typically int and assigned by the system (vanilla/tml, PfE in this case) automagically. Ideally Type is an int of its own for every prefix (every prefix should have an identifier) and another property to which group it belongs
There was a problem hiding this comment.
I hope you're not syncing by string comparison (string is 4+2*c bytes vs 2 bytes from ushort for example)
|
|
||
| public override void AI(NPC npc) | ||
| { | ||
| //todo: shouldn't these be in the OnCreate? |
There was a problem hiding this comment.
Yes, they should be in OnCreate
| npc.buffImmune[BuffID.OnFire] = false; | ||
| npc.buffImmune[BuffID.CursedInferno] = false; | ||
|
|
||
| //todo: should this really trigger twice, if both buffs apply? |
There was a problem hiding this comment.
My vote is no. Roll a single random that determines if a debuff should be applied, then a separate random to decide which one.
| Main.projectile[p].hostile = true; | ||
| Main.projectile[p].friendly = false; | ||
| Main.projectile[p].timeLeft = 90; | ||
| Main.projectile[p].ai[0] = Main.rand.Next(-6, 13) / 2; |
There was a problem hiding this comment.
This kind of stuff can easily be synced by setting projectile.netUpdate (or projectile.netUpdate2 iirc) to true, will make it sync AI by itself (have these fields be chosen if Main.netMode != 1 due to non-determinism)
|
|
||
| public override void AI(NPC npc) | ||
| { | ||
| if (npc.target == 255 || Main.netMode == 2) |
There was a problem hiding this comment.
Seen netMode == 2 checks in many places. They should not exist in AI this way. Server has all the right to know an NPCs (prefixed) state and since often stuff might even need to be synced, server can never even reach those points and send packets. Makes sense aye?
| target.inventory[i].stack -= num3; | ||
| if (target.inventory[i].stack <= 0) | ||
| { | ||
| target.inventory[i] = new Item(); |
There was a problem hiding this comment.
item.TurnToAir() is encouraged
| { | ||
| damage += target.statMana / 4; | ||
| target.statMana /= 2; | ||
| CombatText.NewText(new Rectangle((int)target.position.X, (int)target.position.Y - 50, target.width, target.height), new Color(20, 20, 120, 200), "" + target.statMana); |
There was a problem hiding this comment.
"" + target.statMana
oof... .ToString() is encouraged
rewrite of enemy prefixes, still beta, WIP and could explode!