Edit: or do you just mean it was a copy-paste gone a bit bad? Just trying to clear it up!
This yeah. XD I was sort of copy pasting the concept from the ships data map- which requires the local object since the data map's value is a map itself.
That being said, even for a primitive if I needed to reassign the value from the data map multiple times without needing to add it back in the map, having a local variable is necessary so I might default to it anyway to be honest - especially if I'm unsure of the exact scope of the class at the start. Imo, it can also be "cleaner" from the perspective of read-ability to do things this way. Definitely not in this case, but you get the idea.
The IDE highlights all instances of variables when clicking on any one instance so that can be invaluable to me for times when the code gets complex. You can also do this with objects/object methods, sure, but if multiple objects of the same class use the same method for different values or the object is the same instance of the class but uses an individual method for different values, etc, it can get trickier/misleading to encompass a single value and its use in the code across the whole class using this technique. So that's another reason I would be tempted to go the local variable route anyway. At least initially. (My old team lead and a couple of teachers I had also
hated anything hardcoded so I'm probably a little brainwashed by now too.)
In the case of the rotation logic, though, once the local variable was just *sitting there* from the copy paste I was kind of "hardwired" to just use it lol. So I set up the if statement to check the local variable instead of realizing immediately that it would be the only instance of that engine data call in the actual logic. Read-ability doesn't matter there and so it makes
much more sense to just skip the local variable all together for that use case and directly add it to the engine data - especially since each assignment requires an update to the data map anyway like you pointed out. (Unlike the ships data map.)
Still, after that first implementation I definitely went too "copy paste crazy" with it lol - which is sloppy. I tend to take an iterative approach to coding and likely would have caught it in the clean up phase. I actually already had it looking like your example before I saw this post due to your earlier comment and by catching the fact that I could directly call the value in the if statement - which made me go "oh this could be way cleaner than it is."
Basically, I make new code as efficient as I can while ensuring functionality and trying to think down the line of flexibility for use, then go back over it and think to myself "how can I scrape out anything unnecessary?" - which can sometimes even cause refactoring of the logic if I notice I can do something in a more efficient way that I didn't initially catch. For instance, if I feel a code block is too big and could be simplified with private helper methods, etc. That one in particular somewhat helps prevent the above use of instance highlighting from being as necessary so I usually don't regret it, hah. That's kind of how my thought process works if that makes sense.
Anyway, thanks for taking the time you did to respond I always appreciate it.
Here's the updated code. (In a spoiler because this post is already huge.) Feel free to poke more holes if you see them as it really helps me learn! I also have a lot more to add to this class, so the more efficient it is the better!
Spoiler
public class CHM_adamantine_consortium extends BaseHullMod {
private static final String ID = "CHM_adamantine_consortium";
private static final float MALFUNCTION_PROB = 0.05f;
private static final float CRIT_MALFUNCTION_PROB = 0.5f;
public static String TEXT_POPUP_INTERVAL_KEY = "text_popup_interval_key";
public static String IS_ROTATING_KEY = "rotating_key";
// Logging components.
private static final Logger LOG = Global.getLogger(CHM_adamantine_consortium.class);
private static Map<HullSize, Float> mag = new HashMap<>();
static {
mag.put(HullSize.FRIGATE, 0.4f);
mag.put(HullSize.DESTROYER, 0.6f);
mag.put(HullSize.CRUISER, 0.8f);
mag.put(HullSize.CAPITAL_SHIP, 1f);
}
public static class PeriodicTextPopupTracker {
IntervalUtil interval = new IntervalUtil(8f, 12f);
}
@Override
public void advanceInCombat(ShipAPI ship, float amount) {
CombatEngineAPI combatEngine = Global.getCombatEngine();
if (!combatEngine.getCustomData().containsKey(ID)) {
combatEngine.getCustomData().put(ID, new HashMap<>());
}
String key = TEXT_POPUP_INTERVAL_KEY + "_" + ship.getId();
PeriodicTextPopupTracker textTracker = (PeriodicTextPopupTracker) combatEngine.getCustomData().get(key);
if (textTracker == null) {
textTracker = new PeriodicTextPopupTracker();
combatEngine.getCustomData().put(key, textTracker);
}
textTracker.interval.advance(amount);
try {
Map<ShipAPI, DreadAuraEffectOrigin> ships = (Map) combatEngine.getCustomData().get(ID);
for (ShipAPI enemy : CombatExtensionsKt.getNearbyEnemies(ship, 1500f)) {
if (!enemy.isDrone() && !enemy.isFighter() && enemy.isAlive()) {
DreadAuraEffectOrigin aura = ships.get(enemy);
if (aura == null) {
aura = new DreadAuraEffectOrigin();
ships.put(enemy, aura);
LOG.info("Adding ship: " + enemy.getName() + " to ships map.");
}
MutableShipStatsAPI stats = enemy.getMutableStats();
float effect = stats.getDynamic().getValue(Stats.DMOD_EFFECT_MULT);
stats.getWeaponMalfunctionChance().modifyFlat(ID, MALFUNCTION_PROB * effect);
stats.getCriticalMalfunctionChance().modifyFlat(ID, CRIT_MALFUNCTION_PROB * effect);
aura.alpha = 1f;
createAura(aura, enemy, combatEngine, amount);
if (!aura.textPopupHappened && textTracker.interval.intervalElapsed()) {
// Scared text popup here.
LOG.info("Text would popup here.");
aura.textPopupHappened = true;
}
}
}
for (ShipAPI enemy : ships.keySet()) {
if (enemy != null && (!enemy.isAlive() || MathUtils.getDistance(ship, enemy) > 1500)) {
DreadAuraEffectOrigin aura = ships.get(enemy);
aura.resetAura(combatEngine, enemy);
if (aura.textPopupHappened && textTracker.interval.intervalElapsed()) {
// Relief text popup here.
LOG.info("Text boolean has been reset.");
}
aura.textPopupHappened = false;
}
}
} catch (ClassCastException e) {
LOG.error(e);
throw new RuntimeException(e);
}
}
public String getDescriptionParam(int index, HullSize hullSize) {
if (index == 0) return "" + "%";
return null;
}
public boolean isApplicableToShip(ShipAPI ship) {
return ship != null && ship.getCaptain().isPlayer();
}
private static void createAura(DreadAuraEffectOrigin aura, ShipAPI origin, CombatEngineAPI combatEngine, float amount) {
float radius;
String rotatingKey = IS_ROTATING_KEY + "_" + origin.getId();
Boolean isRotating = (Boolean) combatEngine.getCustomData().get(rotatingKey);
if (isRotating == null) {
combatEngine.getCustomData().put(rotatingKey, false);
}
if (!(Boolean) combatEngine.getCustomData().get(rotatingKey)) {
radius = origin.getShieldRadiusEvenIfNoShield() * 1.5f + 50f;
aura.sprite.setSize(radius, radius);
aura.sprite.setAlphaMult(aura.alpha);
aura.sprite.setAngle(aura.angle);
aura.angle += 0.1f * amount;
if (aura.angle > 10000f) {
aura.angle -= 10000f;
}
combatEngine.getCustomData().put(rotatingKey, true);
LOG.info("Aura created. Rotating logic engaged.");
} else {
aura.sprite.setAngle(aura.angle);
aura.angle += 1;
}
}
public final static class DreadAuraEffectOrigin {
public float alpha;
public float angle;
public boolean textPopupHappened;
public SpriteAPI sprite;
public DreadAuraEffectOrigin() {
alpha = 0f;
angle = 0f;
textPopupHappened = false;
sprite = Global.getSettings().getSprite("misc", "dread_aura");
}
public void resetAura(CombatEngineAPI combatEngine, ShipAPI origin) {
String rotatingKey = IS_ROTATING_KEY + "_" + origin.getId();
combatEngine.getCustomData().put(rotatingKey, false);
sprite.setAlphaMult(0f);
alpha = 0f;
}
}
}
*EDIT* Tiny nitpick here, but for the sake of other modders looking at this:
Should just be:
public void resetAura(CombatEngineAPI combatEngine, ShipAPI origin) {
combatEngine.getCustomData().put(rotatingKey, false);
sprite.setAlphaMult(0f);
alpha = 0f;
}
- is missing:
String rotatingKey = IS_ROTATING_KEY + "_" + origin.getId();
Since that is dependent upon the origin as a safeguard, and "rotatingKey" is not global and is not passed into the method.
Author
Topic: Misc modding questions that are too minor to warrant their own thread (Read 2048390 times)