Repository navigation
Conversation
|
/review |
There was a problem hiding this comment.
Refactor direction is good: hoisted blockade/enemy checks, per-ship and per-fleet ork costs, global fleet cap, var _growth instead of leaking growth, and var i fix in orks_end_turn_growth are all improvements. Requesting changes for one blocker: the fleet-arrival ork deposit is now a discarded expression, so non-warboss arrivals add zero orks (regression vs old p_orks += ...). Also needs a guard so ship-expansion does not run with (noone), and the warboss-boarding check should not reuse a stale fleet handle that may have just been ordered to leave. Otherwise readable and close.
|
/review |
There was a problem hiding this comment.
LGTM - both incremental fixes resolve the prior blockers: fleet-arrival ork deposit is now assigned (p_orks += max(2, ...)) and ship-expansion is guarded by _ork_fleet != noone, so no more with (noone) or discarded-expression regression. Verified in full file context; remaining warboss-boards-departing-fleet nuance is benign (instance still valid, no crash) and can be a follow-up if desired.
|
/oc Can you check the diff and write a game balance affect report with how things changed in this PR, in terms of various probabilities and game logic (before VS now). Only changes observable by the players. |
Ork balance report for #1636 (
|
| if (instance_exists(obj_p_fleet)) { | ||
| var ppp = instance_nearest(x, y, obj_p_fleet); | ||
| if ((point_distance(x, y, ppp.x, ppp.y) < 50) && (ppp.action == "")) { | ||
| _blockaded = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
| if (instance_exists(obj_p_fleet)) { | |
| var ppp = instance_nearest(x, y, obj_p_fleet); | |
| if ((point_distance(x, y, ppp.x, ppp.y) < 50) && (ppp.action == "")) { | |
| _blockaded = true; | |
| } | |
| } | |
| _blockaded = system.has_orbiting_player_fleet(); |
function that was going to ship in my mission rework but i see it can be used here so i've just merged it into main on it's own little pr
|
i'm fine with pretty uch all this but i would suggest the below be reverted to the previous values, strength 2 orks will almost always get removed by pdf on major worlds and as strength 2 orks never grow this means functionally i think this may end up overcorrecting to making orks very un threatening. |
The problem iirc was (in my head), that Together with the rest of the changes tho, it may no longer be such a problem. I may in fact revert it, because this change was done before everything else. I've marked it as draft, because I basically went to rewrite a lot of the said method anyway. |
|
|
||
| if (_alert_triggered) { | ||
| if (!_fleet_persists) { | ||
| if (!instance_exists(_ork_fleet)) { |
There was a problem hiding this comment.
the instance does not get destroyed till the end of the event nor does it get deactivated. This will always treat the ork fleet as existing unless you explicitly deactivate it or use the flag, imo why not just keep the separate flag
There was a problem hiding this comment.
doesn't it get destroyed at line 182?
|
/review |
There was a problem hiding this comment.
Incremental look at the three new commits (ork rework iterations + luck re-roll change).
Both previously blockers are resolved: the fleet-arrival ork deposit is now assigned through edit_forces, ship expansion is guarded by _ork_fleet != noone, and the arrival alert checks instance_exists(_ork_fleet).
The level/men model (level n = 2^(n-1) men, bit_length as inverse) is used consistently in arrival, spread and spores; boarding/embark ordering keeps is_garrison_force initialized (ai_b runs before ai_c).
Luck changes are consistent: always roll twice and keep min/max, with player_benefit_at direction correctly applied for "high" and "low" callers everywhere incl. roll_dice_unit.
No functional blockers found, but the spore seeding and per-level production thresholds are significant balance decisions (spores ignore ownership/enemies/blockade, ships effectively require 5+ orks without discounts), so leaving this to the maintainers.
| } | ||
|
|
||
| function fleet_power_value(_fleet) { | ||
| if (_fleet == noone || !instance_exists(_fleet)) { |
There was a problem hiding this comment.
Per the repo's minimal-defensive-code rule: both callers (ork_fleet_arrive_target, ork_planet_turn via scr_orbiting_fleet) pass either noone or a live instance handle, so !instance_exists(_fleet) is not reachable. The noone check alone covers the only real case, and the arrival call site guarantees a valid fleet either way. Suggest dropping the instance_exists guard or the whole guard if callers are expected to validate.
| return 0; | ||
| } | ||
| var _result = 1; | ||
| for (var i = 1; i < _index; i++) { |
There was a problem hiding this comment.
Minor: the doc already states this equals 2^(_index - 1), so the loop can be return power(2, _index - 1);. Saves the O(n) loop and a local. bit_length likewise mirrors log2 for powers of two, though the careful loop version there is fine.
| var _spores_roll = roll_dice_chapter(1, 100, "low"); | ||
| var _spores_theshold = 100 - (10 * planet_forces[eFACTION.ORK]); | ||
|
|
||
| if (_spores_roll >= _spores_theshold) { |
There was a problem hiding this comment.
Design question: free spores now run with no checks at all on source owner, planet enemies, or system blockade (the old code required the source planet to be ork-owned and enemy-free). An infestation on a garrisoned imperial planet can now silently seed 1 ork per turn onto every zero-ork planet in the system, including player-held worlds, bypassing _system_blockaded. If that is the intended lore/balance (spores everywhere), fine, but please confirm it is deliberate and not just a missing !_planet_has_enemies condition like the other blocks. Nit: _spores_theshold -> _spores_threshold.
|
|
||
| if (!_planet_has_enemies && _can_build && planet_forces[eFACTION.ORK] >= 4) { | ||
| var _production_roll = roll_dice_chapter(1, 100, "low"); | ||
| var _production_threshold = 80 + ((6 - planet_forces[eFACTION.ORK]) * 10); |
There was a problem hiding this comment.
Confirm the intent: with no discounts, a 4-ork planet gets a threshold of 100, so vanilla 4-ork planets can essentially never build a fleet (roll >= 100); only Hive/Forge/stronghold/warboss discounts make it reachable. If the requirement is really "at least 4 orks", the base may want to be lower (e.g. 60 + (6 - orks) * 10); as-is the summary's claim of 4 orks is only technically true.
Summary by cubic
Reworks ork growth and ship building so fleets only spawn from safe, unblockaded planets with enough orks, and now cost ork forces to create or expand. Fixes ork fleet arrival not adding forces to the target planet. Reworks luck rolls to roll twice and keep the better result for player-beneficial outcomes, and the worse for player-detrimental ones.
Bug Fixes
neworkfleetcheatcode spawning an empty fleet; it now spawns with ships.Shitty LuckandGreat Luckso the discarded roll is the same as before.Written for commit a81e715. Summary will update on new commits.