Fractal Softworks Forum

Please login or register.

Login with username, password and session length
Advanced search  

News:

Starsector 0.97a is out! (02/02/24); New blog post: Simulator Enhancements (03/13/24)

Pages: [1] 2 3 ... 11

Author Topic: Random Code Mistakes Thread  (Read 36141 times)

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Random Code Mistakes Thread
« on: November 01, 2014, 11:27:25 PM »

This thread should be a dumping ground for various coding mistakes we catch.  I'll start off with one I caught today:

Line 64 of MercAndPirateFleetManager.java
Code
activeFleets.remove(remove);
should be
Code
activeFleets.removeAll(remove);

Otherwise, there is a potential memory leak in rare cases.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #1 on: November 01, 2014, 11:33:31 PM »

Ugh, thanks for spotting that - fixed.
Logged

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Re: Random Code Mistakes Thread
« Reply #2 on: November 03, 2014, 07:34:43 PM »

(Note: similar lines in the other FleetManager java files are messed up as in the OP)

Found a logical error on line 246 of FleetFactory.java:
Code
if (tier >= 2) {
faction.pickShipAndAddToFleet(ShipRoles.COMBAT_FREIGHTER_SMALL, qf, fleet);
} else if (tier >= 3) {
faction.pickShipAndAddToFleet(ShipRoles.COMBAT_FREIGHTER_MEDIUM, qf, fleet);
}

The second scope is unreachable.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #3 on: November 03, 2014, 07:51:59 PM »

Thank you again - fixed these up.
Logged

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Re: Random Code Mistakes Thread
« Reply #4 on: November 03, 2014, 08:19:21 PM »

Also just noticed another, on line 263 is a random if statement with no scope (i.e. it creates a scope at compilation over the next line, which is another if statement, it turns out).  I believe it's supposed to be deleted, based on the rest of the code.
Code
if (stability < (float) Math.random() * 10f)

Man, you find all sorts of stuff when you have to analyze another person's code.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #5 on: November 03, 2014, 08:34:32 PM »

... wow. Yeah, deleted.

Man, the sort of stuff people find when they're looking at your code :)
Logged

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Re: Random Code Mistakes Thread
« Reply #6 on: November 06, 2014, 12:31:20 AM »

Oh boy, this one looks like it could be a problem.  Line 145 of CoreCampaignPluginImpl.java:

Code: java
if (Global.getSector().getIntel().getCommSnifferLocations().contains(this)) {

"this" should be "entity".
Logged

Tartiflette

  • Admiral
  • *****
  • Posts: 3529
  • MagicLab discord: https://discord.gg/EVQZaD3naU
    • View Profile
Re: Random Code Mistakes Thread
« Reply #7 on: November 06, 2014, 05:01:30 AM »

I don't know where is the problem in the code, but
public interface OnHitEffectPlugin {
   void onHit(DamagingProjectileAPI projectile, CombatEntityAPI target, Vector2f point, boolean shieldHit, CombatEngineAPI engine);
}
don't detect shieldHit properly anymore.

[EDIT] False alarm, sorry about that, but it wasn't really my fault, I swear! Apparently Netbean don't really recompile your code if you don't modify your file, but use the previously compiled version. I have copy-pasted the exact same code from an old file into a new one, and it work in the new file, not in the old..... I should probably clear some cache or something to avoid strange issues like that.
« Last Edit: November 06, 2014, 09:48:50 AM by Tartiflette »
Logged
 

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #8 on: November 06, 2014, 09:01:51 AM »

Oh boy, this one looks like it could be a problem.  Line 145 of CoreCampaignPluginImpl.java:

Code: java
if (Global.getSector().getIntel().getCommSnifferLocations().contains(this)) {

"this" should be "entity".

Fixed, thank you - actually, just removed that code, due to some other changes, but yeah, this was what was causing the "can install comm sniffer many times in a row" bug.


I don't know where is the problem in the code, but
public interface OnHitEffectPlugin {
   void onHit(DamagingProjectileAPI projectile, CombatEntityAPI target, Vector2f point, boolean shieldHit, CombatEngineAPI engine);
}
don't detect shieldHit properly anymore.

(Probably doesn't belong in this thread, then, since it's specifically for bugs in exposed implementation code.) That said, what's "properly" and are you sure? That code hasn't changed in a while. Also, as evidenced by some quick testing with the Ion Cannon effect, it seems to work.
Logged

Darloth

  • Admiral
  • *****
  • Posts: 592
    • View Profile
Re: Random Code Mistakes Thread
« Reply #9 on: November 06, 2014, 01:33:52 PM »

Tiny one from me:

Line 577-559 of FleetEncounterContext is a case statement for Status.REPAIRED without a corresponding break; I'll include the whole select as it's quite small.

Code: java
public float getSalvageMult(Status status) {
float mult = 1f;
switch (status) {
case DESTROYED:
mult = 0.5f;
break;
case DISABLED:
mult = 1f;
break;
case REPAIRED:
mult = 1f;
case CAPTURED:
mult = 1f;
break;
}
return mult;
}

Right now it won't be a bug, because mult is set to the same thing, but it could be later maybe if you somehow don't notice it.

Also in FleetEncounterContext:

lootedCargoCapacity, lootedFuelCapacity, maxLoserCargoCapacity and maxLoserFuelCapacity are assigned to after some calculation, but never actually used anywhere I can see.
« Last Edit: November 06, 2014, 01:38:09 PM by Darloth »
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #10 on: November 06, 2014, 01:38:10 PM »

Thank you - "fixed".
Logged

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Re: Random Code Mistakes Thread
« Reply #11 on: November 06, 2014, 01:51:50 PM »

EndConversation and OpenComms reference CoreFleetInteractionDialogPluginImpl directly, making it impossible to replace that plugin without breaking conversations.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #12 on: November 06, 2014, 02:11:18 PM »

EndConversation and OpenComms reference CoreFleetInteractionDialogPluginImpl directly, making it impossible to replace that plugin without breaking conversations.

Those fields are public, so it's possible.
Logged

Dark.Revenant

  • Admiral
  • *****
  • Posts: 2806
    • View Profile
    • Sc2Mafia
Re: Random Code Mistakes Thread
« Reply #13 on: November 06, 2014, 02:23:30 PM »

EndConversation and OpenComms reference CoreFleetInteractionDialogPluginImpl directly, making it impossible to replace that plugin without breaking conversations.

Those fields are public, so it's possible.

Ah, I forgot that they were static.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: Random Code Mistakes Thread
« Reply #14 on: November 06, 2014, 02:25:38 PM »

It'd probably make more software-design-sense if those fields were in OpenComms/EndConversation, respectively, but, you know. If I "fixed" that now, that'd just cause modded code to not compile for an imo not-good-enough reason.
Logged
Pages: [1] 2 3 ... 11