Galaxy-Spam commented 4 years ago

Refractor supply mule code to reduce copy & paste using . Possible issues

[x] Due to a possible frame delay between checking for trade offers and ordering ships to trade the trade offer might change. This should be mitigated by checking the availability before issuing the order (like egosoft did)
[x] Check savegame compability using blocking actions (we might be able to just restart the script on version update to resolve all possible storage issues) See https://forum.egosoft.com/viewtopic.php?t=363625

Possible changes

[x] Buy from multiple stations, but only sell to a single station
[x] Do we actually need this? What if we can sell the same ware to a different station? Otherwise why not remove the selloffer from the list entirely?
```

<do_if value="$supplyTrades.indexof.{$someSupply}">
<continue />
</do_if>
```
[ ] Once tradeoffer are issued, $OccupiedCargo should be redundant, since x4 must store this information somewhere already for vanilla trades.
[ ] Possibliy store $debugchance, $debugDirName and $debugFileName in a global or on the ships blackboard
[x] is the $specialWareBasket respected at all for supply mules buyoffers?
[x] Should we add wares to the $specialWareBasket at all? Does it help with claritiy or does the opposite?
[x] Currently, if one order is issued for a single case, but the cargo space is not filled, the supply mule will just stop searching for other orders. Instead it should continue to search for the same buyer and try to fill its cargo
```
<do_if value="this.ship.orders.count gt 0">
 <resume label="end" />
</do_if>
```
[x] How to determine the currently serving station when the mule is not assigned. By maximum profit for trading a single station in a single run?
[ ] Could we sort trades for wares by priority?
[ ] Can we merge sell trades?

[x] If we buy from NPCs for our stations, gate distance it totally random...

<match_gate_distance object="$playerBuildNeeds.random.owner" min="0" max="$maxDist">

bhayden53 commented 4 years ago

We already do check the availability of the trade btw:

<do_if value="$bestProfit gt 0">
    <do_if value="$bestSupplyTrade.available and $bestNeedTrade.available">
        <debug_to_file chance="$debugchance" name="$debugFileName" directory="$debugDirName" text="'buying ' +$bestAmount + ' ' +$bestNeedTrade.ware +' from ' +$bestSupplyTrade.owner.knownname
        +' at ' +$bestSupplyTrade.unitprice.formatted.{'%s %Cr'} + ' to sell to ' +$bestNeedTrade.owner.base.knownname
        +' at ' +$bestNeedTrade.unitprice.formatted.{'%s %Cr'} + ' for a profit of ' +$bestProfit.formatted.{'%s %Cr'} " />

        <write_to_logbook category="upkeep" title="$logbookEntryTitle" interaction="showonmap" money="$bestProfit" text="'buying ' +$bestAmount + ' ' +$bestNeedTrade.ware +' from ' +$bestSupplyTrade.owner.knownname
        +' at ' +($bestSupplyTrade.unitprice.formatted.{'%s %Cr'}) + ' to sell to ' +$bestNeedTrade.owner.base.knownname
        +' at ' +($bestNeedTrade.unitprice.formatted.{'%s %Cr'}) + ' for a profit of ' +($bestProfit.formatted.{'%s %Cr'})" />

        <create_trade_order name="$bestOrder" object="this.ship" tradeoffer="$bestSupplyTrade" amount="$bestAmount"  immediate="true"/>
        <create_trade_order object="this.ship" tradeoffer="$bestNeedTrade" amount="$bestAmount" />
    </do_if>
    <do_else>
        <continue />
    </do_else>

bhayden53 commented 4 years ago

btw, the debug_to_file and write_to_logbook have to occur before the trade order, otherwise the prices that are printed are affected by the orders themselves. Unfortunately that still leaves a race condition but it's pretty small.

Galaxy-Spam commented 4 years ago

While looking on how to restructure the code, i don't really get why we have $maxtrades. So my current reading of the implementation is, that we have the following: Example: "Station A" buys "Silicon Wafer" and "Refined Material" "Station B" sells "Silicon Wafer" "Station C" sells "Refined Material" We then trade "Station B buy" -> "Station A sell" -> "Station C buy" -> "Station A sell" So what do we gain here? Always evaluating 1 trade would yield the same result, or? "Station B buy" -> "Station A sell" -> restart script -> "Station C buy" -> "Station A sell" The intention would be (i assume): "Station B buy" -> "Station C buy" -> "Station A sell", where does that happen?

Galaxy-Spam commented 4 years ago

never mind, just figured it out

<create_trade_order name="$bestOrder" object="this.ship" tradeoffer="$bestSupplyTrade" amount="$bestAmount"  immediate="true"/>
<create_trade_order object="this.ship" tradeoffer="$bestNeedTrade" amount="$bestAmount" />

bhayden53 commented 4 years ago

Maxtrades also sets the minimum allowed cargo that makes a valid trade. It's a bit of a relic from the old code, and I decided to keep it. We can rethink the method of picking multiple trades at some point, but maybe we should try a direct functionality port first, to avoid changing too many things at once.

In particular, I'm not that happy with the fact that technically you could buy from multiple suppliers and sell to multiple need-ers. It doesn't seem to happen very often though. Also you can end up going all over the place in a very non-optimal order. But I didn't want to put traveling salesman into mules so...

bhayden53 commented 4 years ago

The main reason for having multiple trades, as I see it, is to go around and pick up the last little bits of junk that are needed to finish off a build storage.

Galaxy-Spam commented 4 years ago

yeah, same thought process here. I already decided to postpone all changes to the code itself after porting. But i might use this issue thread to create a list of stuff that could be optimized.

Misunderstood-Wookiee commented 4 years ago

Tightly sits inside objective #6

Galaxy-Spam commented 4 years ago

Should (at the end) resolve #6 for the case of the supply mule. For now it's working, with the exception of some debug messages and there is probably some other stuff i missed. I will check out the run_script savegame compatibility for now.

Galaxy-Spam commented 4 years ago

Savegame safety seems mostly fine. I created 2 savegames for testing. Savegame A has a fleet of currently running mules Savegame B has the same fleet but has one mule stuck in a 60s blocking wait command.

When loading these games without the extension, we only get the default error messages

[General] 259626.11 ======================================
[=ERROR=] 259626.11 Error on AI director import: AI script name 'supplymule' not found
[General] 259626.11 ======================================
[General] 259626.11 ======================================
[=ERROR=] 259626.11 Error on AI director import: AI script name 'travelmule' not found
[General] 259626.11 ======================================

For the next step i removed the wait command, and insert a patch code to reissue the order on load.

<param name="restartScript" type="internal" default="false"/>

<patch sinceversion="3">
    <!-- This will reissue any standing supply mule orders -->
    <!-- it should mitigate any problems with blocking actions beeing updated inbetween savegames -->
    <!-- It might not ensure help with removing this script from a savegame -->
    <debug_text text="'PATCH: Restarting supply mule script on load for version safety on mule %1'.[this.ship.idcode]" filter="savegame"/>
    <edit_order_param order="this.assignedcontrolled.order" param="'restartScript'" value="true"/>
</patch>

When using /refreshai this leads to an error (and does not reload the script).

[General] 259705.07 ======================================
[=ERROR=] 259705.07 Error while refreshing AI script supplymule, attention 'unknown', line 253: Blocking action 'run_script' has invalid version, expected 'wait' instead, previously in line 229. Missing sinceversion attribute?
[General] 259705.07 ======================================
[=ERROR=] 259705.07 Error while refreshing AI script supplymule, attention 'unknown': Missing blocking action 'run_script', previously in line 579, since version 2
[General] 259705.07 ======================================
[General] 259705.07 ======================================

This is because /refreshai does not execute any patch commands. Note: loading any other savegames in the same instance of X4 will fail. Saving any games could/might corrupt the affect new save file.

If loading the savegame with the updated script, i will work fine for both cases. For the supplymule in the 60s wait, it will use the patch and restart the command(now without wait) and will operate normally.

[General] 259626.11 ======================================
[Savegame] 259626.11 *** aicontext<supplymule,0x25f25>: PATCH: Restarting supply mule script on load for version safety on mule XLB-179
[General] 259626.11 ======================================

I do have problems loading 2 savegames in a row (so loading a savegame ingame after loading the first). But my savegame is likly messed up by now, so i doubt it has anything to do with this. But i will need to test this.

Changes are pushed to current PR to dev.

Galaxy-Spam commented 4 years ago

No loading problem when using fresh saves. So it should be fine.

bhayden53 commented 4 years ago

fyi i have this PR checked out locally and I dont have anything else going at the moment, so I'm not rushing to merge into dev. let me know if you need me to for some reason

bhayden53 commented 4 years ago

pushed a few minor bug fixes from things I noticed in the debug log.

Something seems to be up with the search for supply orders at the moment. Right before finishing for the night I noticed this:

Current priority is to supply "Nope's Wharf (RUX-375)" by buying "resources" from "player"
------------------------------------------------------------------------------------------------------------------------------------
Currently Serving: Nope's Wharf
----- needs (15):
Nope's Wharf wants to buy 196 Advanced Composites from 0x16282e for 54481 (0.044537)
Nope's Wharf wants to buy 19 Advanced Electronics from 0x16282e for 81340 (-0.659868)
Nope's Wharf wants to buy 123 Antimatter Converters from 0x16282e for 28525 (-0.648585)
Nope's Wharf wants to buy 1 Drone Components from 0x16282e for 76276 (-0.660437)
Nope's Wharf wants to buy 176766 Energy Cells from 0x16282e for 1000 (-1)
Nope's Wharf wants to buy 3054 Engine Parts from 0x16282e for 15020 (-0.588889)
Nope's Wharf wants to buy 60 Field Coils from 0x16282e for 31099 (-0.612182)
Nope's Wharf wants to buy 2271 Hull Parts from 0x16282e for 18882 (-0.320317)
Nope's Wharf wants to buy 540 Medical Supplies from 0x16282e for 6058 (-0.235652)
Nope's Wharf wants to buy 14316 Missile Components from 0x16282e for 766 (-0.446667)
Nope's Wharf wants to buy 7 Scanning Arrays from 0x16282e for 91367 (-0.660332)
Nope's Wharf wants to buy 75 Shield Components from 0x16282e for 14011 (-0.638533)
Nope's Wharf wants to buy 953 Smart Chips from 0x16282e for 4982 (-0.652727)
Nope's Wharf wants to buy 271 Turret Components from 0x16282e for 21820 (-0.502752)
Nope's Wharf wants to buy 2956 Weapon Components from 0x16282e for 25363 (-0.275175)
----- supply offers (253):
HOP Missile Component Factory I wants to sell 766 Missile Components to null for 1293 (0.9825)
HOP Hull Part Factory II wants to sell 1866 Hull Parts to null for 25758 (0.771111)
HOP Solar Power Plant I wants to sell 86214 Energy Cells to null for 1278 (-0.536667)
HOP Hull Part Factory I wants to sell 5145 Hull Parts to null for 20148 (-0.119365)
HOP High Tech Factory III wants to sell 2629 Hull Parts to null for 24924 (0.63873)
HOP High Tech Factory III wants to sell 1935 Medical Supplies to null for 7482 (0.383478)
HOP Refined Goods Complex I wants to sell 8012 Medical Supplies to null for 6396 (-0.0886956)
HOP Refined Goods Complex II wants to sell 2885 Medical Supplies to null for 7249 (0.282174)
ARG Hull Part Factory I wants to sell 11365 Hull Parts to null for 21348 (0.0711111)
ARG Missile Component Factory I wants to sell 10484 Missile Components to null for 945 (0.1125)
TEL High Tech Factory II wants to sell 2980 Smart Chips to null for 6771 (0.8925)
...

It looks like in the search for suppliers the $factions variable is never used. It's used in the search for needs though.

Galaxy-Spam commented 4 years ago

supply mules should now respect the $factions setting, see aee880e8dbb52be73d6c46ab01bf5448a28f7945

Galaxy-Spam commented 4 years ago

61 should now be in a functional beta state.

Priority now is to find any remaining bug and get ready for 3.2 publish. (Note: This current branch is only working in 3.2)

Misunderstood-Wookiee commented 4 years ago

I merged dev branch into a perm Public Beta main tree. added a feedback template for public beta.

both of which are for times when the project supersedes the current official live patch from egosoft and we need to push things or do things specifically for a public beta. https://github.com/Misunderstood-Wookiee/Mules-and-Warehouses-Extended/tree/PublicBeta

bhayden53 commented 4 years ago

@Galaxy-Spam if you're satisfied with your checklist, let's close this issue with the release of mules 6.1.0 and make individual issues for anything big enough that you'd still like to finish up.

Galaxy-Spam commented 4 years ago

I think we can close this, i will open small issues once i have more time to code again.

Misunderstood-Wookiee / Mules-and-Warehouses-Extended

Refractor Supply Mule Code #57

61 should now be in a functional beta state.