Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback #2

Open
lyravega opened this issue Mar 10, 2023 · 13 comments
Open

Feedback #2

lyravega opened this issue Mar 10, 2023 · 13 comments

Comments

@lyravega
Copy link

Hiya! I simply love this mod and have some feedback. Posting here as well as the Nexus page, not sure which one would get to you.

• Automatic reset of station accounts: If the buy orders aren't restricted in order to let the NPC traders sell to a Warehouse, due to automatic reset no buy orders will be put out. If this was an option, ideally something like a "transfer account at", where 0 disables this behaviour, it'd be great. It'd also make the mod get along friendly with "Finance Hub: Transfers". Though this is a minor thing; I was hoping to let NPC traders also sell to the Warehouses and lift some weight off of my own Warehouse Fleet.

• Dock when Idle: This is a nice feature, keeps the fleet protected when idle and map clean of... stuff flying everywhere. However I noticed that any large subordinates will keep docking/undocking, but as a visual glitch; their orders keep changing all the time. However, if I assign them as individuals instead of subordinates, they don't do this.

• Force Station Account: I understand your reasoning behind this, why the ships use your account instead of station's. However, there's a trick and also a mod that allows you to copy default orders, even when a ship is assigned to a station. Such an option might be useful in those cases, but this is more of a personal suggestion to be honest; you cannot account for any mods that might be out there, and do things accordingly.

• XL Ships?!?: I have been having trouble with XL ships, most specificly, the Erlking. I set everything up for the Erlking, and was trying to use it as a Warehouse-to-Warehouse trader mainly, as it is more than capable of obliterating anything in its path and as it has the biggest cargo space of all ships. However, it didn't work at all, and just kept docked at the warehouse. When I used the very same setup on a L fleet, they immediately started working. Is the mod only affecting S M L but not XL ships?

Anyway, thanks for the great mod! Even just for trading alone it solved many of my issues!

@lyravega
Copy link
Author

lyravega commented Mar 10, 2023

Hi again! I forked the project and made some adjustments. However, it is incomplete as my modding skills in X4 have wide gaps in knowledge.

Main changes are:

  • Added XL ship support; XL ships are no longer ignored and use shared and XL target lists
  • Renamed "Account Threshold (Million)" to "Station Account Threshold (M)"
  • Added a toggle for automatic station-to-player transfers; makes the mod kinda compatible with other mods doing similar, also leaves the station with cash to buy things with
  • Added a slider for maximum station account threshold; the default transfers that happened at 200k now happens at this value
  • Added a toggle for automatic player-to-station transfers; it does nothing for now
  • Added a slider for minimum station account threshold; when a transfer happens, this mount will be left at the station
  • Removed the Warehouse's sector from default list; it was sometimes getting double-added for some reason, additionally not having it as the default allows using only S ships for in-sector logistics

image

Problems:

  • The "player-to-station transfers" toggle does nothing for now. The idea is bringing the station's cash reserves up if it goes down below the set value. This is more of a personal thing for me though, as I am using a trick to add these WarehouseFleet's as station subordinates, as then they start spending the station's cash. So, not really a problem in a way, but even without my specific use case, might be helpful. That is, if you are not using a mod like "Financial Hub: Transfers". But considering that it'll require additional checks for whether the station have enough money, might not be a good idea after all. Anyway...
  • I assume order sync happens when a new ship is assigned as WarehouseFleet. If that is the case, no problem in sync, otherwise it doesn't sync when "Confirm" is pressed. Unsure when this happens as I've mentioned. In any case, looking at the "Extension Options" mod-tool to maybe move all of these synchronized settings over there!
  • The added "Station Min Threshold" can go over "Station Max Threshold" :D I need to learn a bit more to properly add checks and drop it below max. Or, ideally, the slider max should be whatever max threshold's current value is. No idea if latter is possible, former definitely is though.
  • I have no idea how things are patched between mod versions. The new things are undefined initially and require an update on the orders. I see that there's a <patch> block but have no idea how that works. I assume it's what I seek.

Anyway, if you are still working on the mod, cherry pick anything you want! Also, in that case, I'd appreciate if you give me a heads-up and let me check how you fixed some of the things I mentioned above, really keen on learning modding in X4 in general!

@mbleichner
Copy link
Owner

Thanks for all the feedback and for the improvements! Unfortunately I don't have a lot of time for modding right now, so I can't go into detail very much. But here are some thoughts and explanations:

  • The whole station account vs player account thing was a total PITA. In my first attempts, I tried to implement the mod using subordinates and station accounts, but this was extremely buggy and had some weird consequences. In the end, I ditched it, separated the fleets from their stations and used the player account for all transactions. The money-transfer feature was added relatively late and isn't well thought out as you have already noticed. I like your suggestion of a two-way transfer. It might be a bit tricky though to get that right in terms of fairness - when there's many warehouses and too little money on the player account.
  • About XL ships: it never even occurred to me to use them in a warehouse fleet... :D
  • About the order sync: it should get triggered from the confirm button. Maybe some recent update broke it..?
  • Restricting inputs to only allow sensible values is quite limited, unfortunately. Afaik, there is no way to restrict a parameter based on the value of another. The only thing you can do is correcting the inputs after you hit the confirm button.
  • The patch blocks refer to the version attribute of the aiscript. Assuming your savegame contains version 1, and you then update the mod to version 3. When you load up the savegame for the next time, the patch blocks for version 2 and 3 will be executed.

@lyravega
Copy link
Author

lyravega commented Mar 11, 2023

Thank you for the reply! Shame about you not having time to mod as much, but life happens, things change, etc... I understand. Still, thank you for the reply =)

  • On the fork that I made, I added in support for "Extension Options" and simply moved those synchronized setting over there along with the new ones I come up with. I still haven't touched transferring from player to stations yet though, it will require me to understand and learn more of this... weird syntax :D To be honest, as long as a mod like the "Financial Hub: Transfers" exists, there's no need to develop this feature that much.
  • I added XL ships, aside from Erlking there isn't much that can properly trade to be honest. However, considering that some mods are adding XL traders and whatnot, supporting them separately seems like future-proofing the mod. Erlking has the biggest capacity of any potential trader, and is the fastest by the way :D
  • The sync wasn't properly working for some reason, especially when I tried to sync from an XL ship. But when I did it from a L ship for example, it was working properly. I looked around but didn't find anything related to ship size that restricts synchronizing the orders. Maybe Erlking is special-special? Anyway as a solution, I moved those synchronized stuff to "Extension Options" as I've mentioned.
  • "Extension Options" also came to the rescue for this issue that I was having; restricting inputs. minThreshold has a slider maximum of maxThreshold, and if anyone sets the maxThreshold below the value of minThreshold, the slider range and value gets readjusted immediately!
  • Thank you for this information, I'm mostly curious about this thing to purge old values and initialize new values wherever necessary. I assume I need to use these <patch> blocks wherever the values need an update? Just want to make it work properly without resetting everything, or potentially damaging the save files. Also, the version you speak of is the version in the md script xml, not in the context xml correct?

Sorry for these silly questions. Just love the mod! If you ever check the fork for any potential merges, I unfortunately destroyed spacing by mistake. Sorry about that lol =)

PS: Here is how it looks like in "Extension Options":
image

As the settings are now global, I removed them from default order itself:
image

@mbleichner
Copy link
Owner

I just took a quick look at your mod - looks awesome!

  • Moving some options over to a separate menu is a really good idea, imo. I didn't think of that. Maybe it would even make sense to move the remaining slider options over there too.
  • The patch blocks are used in the aiscripts, not the mdscripts. The are used to adjust the order settings after an update. Lets say for example that you have a setting storing a distance in meters. For the next update, you decide it's more convenient to store kilometers instead. You would write a patch block for this doing the conversion, so that the player doesn't have to fix it manually.
  • I didn't see any obvious problems when I loaded up your version. All my settings still seemed correct. So it looks there's no patch blocks needed for now.
  • Can you summarize what already works and what doesn't? I'd really like to see these changes published when they are finished and tested enough. If you are interested, I could also give you permissions on Nexus.

@lyravega
Copy link
Author

lyravega commented Mar 24, 2023

Sorry for late reply, busy week!

Starting with your last point, everything except cash transfers to stations works. There's an option and a slider for it with values ready to go, but I haven't implemented the option. The slider is being utilized for transfers to player though; when a transfer to player occurs, the last slider acts as the minimum, all but that amount is transferred to player.

The other slider options, I didn't move them in that menu as players might want to set them up differently in some cases, though for me they're mostly always the same, except the cargo use percentage. Depending on the ship or use case, I set it differently for some fleets, but maybe the rest could go into that menu as well.

About patches, I'll have to patch something sometime, I think. There are a few null list errors for the new list. It's harmless but I believe I'll need to do it like how you patched the supply targets. I left them there for now, should be an easy fix, and it's a harmless error anyway.

Right now, I am trying to figure out a way to prevent two things from happening:

  • The first one is related to cargo-emptying mechanic. Sometimes trade deals fail because the game feels like it, and that mod mechanic sells the cargo. I've been trying to think of a solution to prevent this, like maybe making the ships return the cargo instead of selling them. Or somehow prevent the trade from failing. "Unable to complete trade." is not really helpful, Egosoft.
  • The other one is related to the docking/undocking problem that you mentioned in your documentation, why you made it a mdscript instead of an aiscript. Anything assisting the head-mule is running an aiscript and keep docking/undocking. Trying to come up with a mdscript to let them assist without using aiscript; maybe assigning an identical 'WarehouseFleet' order when a ship receives an assist order for a 'WarehouseFleet' and replacing the assist with that identical one.

This syntax is driving me nuts though, half of the time I'm looking up at other scripts to figure things out that aren't documented in .xsd files. Like, what properties does a ship have? :D Kudos to you and all other modders that dabble in these scripts to be honest.

As far as personal testing goes, anything I extended seems to be performing as expected; there isn't anything that breaks the game or the mod, and the issues I'm having is mostly related to the game and not the mod I believe. I've been playing with these changes for several days now and everything works as expected, aside from the issues I noted above.

Thank you for offering Nexus permission, but I'm doing minor modifications to your mod, while trying to understand how scripting works in this game. So instead of that, vetting my changes / additions, fixing anything broken, improving things lacking, cherry picking anything you want and adding it yourself would be best for the mod in my opinion!

@mbleichner
Copy link
Owner

The first one is related to cargo-emptying mechanic. Sometimes trade deals fail because the game feels like it, and that mod mechanic sells the cargo. I've been trying to think of a solution to prevent this, like maybe making the ships return the cargo instead of selling them. Or somehow prevent the trade from failing. "Unable to complete trade." is not really helpful, Egosoft.

Yes, this is absolutely a (stupid) workaround because some trades fail for no reason and I don't want my ships clogged up with random stuff.

The other one is related to the docking/undocking problem that you mentioned in your documentation, why you made it a mdscript instead of an aiscript. Anything assisting the head-mule is running an aiscript and keep docking/undocking. Trying to come up with a mdscript to let them assist without using aiscript; maybe assigning an identical 'WarehouseFleet' order when a ship receives an assist order for a 'WarehouseFleet' and replacing the assist with that identical one.

I don't know If I understand correctly what you're saying here, but if you use the mimic behavior, this shouldn't happen. Every ship of the fleet should behave identically in that regard and stay docked if there is nothing to do. At least that's what they do in my game.

This syntax is driving me nuts though, half of the time I'm looking up at other scripts to figure things out that aren't documented in .xsd files. Like, what properties does a ship have? :D Kudos to you and all other modders that dabble in these scripts to be honest.

Actually it's documented surprisingly well. Have you followed the instructions for unpacking the game files?(https://forum.egosoft.com/viewtopic.php?f=181&t=402452)
The most important file is the libraries/scriptproperties.xml - it contains the structure of most game objects along with decent descriptions.
Also the Mission Director Guide is really helpful in case you haven't read it yet.

@lyravega
Copy link
Author

lyravega commented Mar 27, 2023

The most important file is the libraries/scriptproperties.xml

Heh, that solves many of the issues that I'm having =) I was trying to do things blindly, in a way. Was using that guide, looking at other scripts and get help from .xsd autocomplete but that properties thing was missing entirely. Now I can properly try to add some stuff, thank you for the heads up!

@lyravega
Copy link
Author

lyravega commented Apr 5, 2023

I finally had some time to play and finished the other checkbox; from player to warehouse money transfers. It checks if the transfer leaves player with enough money, just like how you check it before the buy orders from non-player stations. Seems to be working nicely as a basic feature, for detailed account management players can use something like "Financial Hub: Transfers" mod, I guess.

About the failed orders and the cargo clean-up routine, I added several more and turned it into an option for testing purposes. The default one is "Refund", a slightly modified version of your own idea. It now checks if the top-level commander is player or a station. If it's a station, refunds to the station's account, otherwise refunds to the player's account. This is more of a personal change though. The other difference is, instead of showing up as "Mission Reward", they show up as a "Transfer".

The new ones are "Teleport", "Destroy", "Return" and "Drop". You can guess what they do from their names. All are based on the "Refund", they just do the routine differently. Personally, I settled on the "Return" one as it seems to be the cleanest one that feels legit if that makes any sense. At first, I tried to "Retry" the failed order, however, I wasn't able to get any info on the failed orders through the scripts, mostly due to my inexperience so settled on the "Return" instead. As soon as an order fails, a return order for the ware is queued up immediately.

image

Played for a good 12 hours or so and everything seems to work as expected. While most of the additions are minor, I'm mostly curious about the "Return" routine; I don't know if this will have any long-term ramifications. One problem that I can think of is, Warehouse may no longer have storage for the wares that the ship is trying to return so checking it might be needed. Other one is, depending on how things work with the mod, it might cause issues that I might not be able to foresee, I need to go through the logs and inspect them to get a proper idea.

If "Return" turns out to be problematic, I'll switch to another, I guess :D But other than that, I think I've added/changed everything I wanted from/for this mod. Rest is optimizing anything that I can, while learning more. Anyway, have a nice day. By the way, any plans for the upcoming DLC? Plans as in, spare time to play it at all? =)

@mbleichner
Copy link
Owner

mbleichner commented Apr 23, 2023

Sorry for the late reply, I was having a really busy time and didn't get to check out your changes until today.

Your code is looking really good so far! Here are my remarks:

  • What happens if transferToStation is enabled, but you don't have enough money to supply all stations? Looking at the code it seems like whenever a ship is scheduled, all available money (above threshold) gets transferred to the station being considered. Does this turn out to be somewhat fair in terms of not overly favoring one station over another?
  • The reason my old code only transfers credits to the player if the station has at least 200k Cr was to reduce loogbook spam. Do you think it would make sense to put it back in? It would probably have to be implemented in both directions (player <-> stations). Not sure if that would work well, though. It could lead to unfair money distributions.
  • About the handling of failed orders: the return policy is probably the most realistic approach; but as you suggested, it needs some kind of escape hatch in case the station has no space to store the wares. Otherwise it could run into an infinite loop. My suggestion is this: add the return trade-order and also add a refund-order (new aiscript) that reliably clears out the cargo space in case the return order failed to unload the wares. So whenever a ship is to be scheduled and contains leftover wares, it will try to return them first and in case this fails, the wares will be refunded. In any case, the actual transport can be carried out as planned.
  • I noticed that you moved all strings to a separate localization file. Is there a documentation of how this works exactly? Can you choose the IDs arbitrarily? What happens if two mods use the same ID?

To continue reviewing/finalising your changes it would be really helpful if you could adjust your formatting to match the original files (2 spaces indentation) and open a pull request on github.

I haven't come around to check out Kingdom End yet, but I'm pretty sure I'll play it eventually (and also implement a few new ideas into the mod? :D)
Have you started it already?

@lyravega
Copy link
Author

lyravega commented Apr 24, 2023

Nah, no worries.

  • It's first come first served. Whomever executes that piece of code first, its warehouse gets all the money above the threshold. Probably could lead to unfair distributions, depending on how busy the warehouses are. To be honest, my initial goal was to disable these auto-transfers so that another mod that do this in greater capacity won't clash with this one. While doing that I wanted to learn more about this game's modding, and instead of simply disabling it, turned it into a toggleable option with configurable limits.
  • The 200k thing is still there but it's disabled initially, and its minimum is 1M now. But yeah, it can lead to unfair money distributions when it's from player->stations, as said above. Easiest solution for fair distribution is counting how many warehouses that the player has and make that piece of script pay every warehouse instead I think, but that'd spam the logbook on its own :D Or the total money can be transferred to a ghost faction, then distributed from there, so the player logbook entry would be singular even though every station may receive something.
  • Yeah, I was planning to use the mod's vanilla refund as a fallback in case the return orders fail. Not sure how to handle / detect failed orders though, are you suggesting some sort of pre-check for the Warehouse's storage first, then refund/return after that? Or a completely new aiscript; a new return order with refund as its failure fallback, so to speak?
  • Not sure if there is a documentation, I used a few other mods as an example; mainly Kuertee's mods. My guess is, if the IDs are the same, I assume mod load order will decide which one will get used. I converted some letters to numbers and used it as the ID, but I can't remember the letters :D It's not an arbitrary choice, but two mods using the same ID may happen for any mod, I think.

If you add ?w=1 to the github URL, whitespaces are ignored in the diff. I plan to fix it though, didn't get around to it. I'll take a last look sometime this week, fix the spacing and open up a pull request. No refunds though! Hahah

I tested the mods and checked if anything needs fixing / updating. Haven't played much of it yet though, been playing a few other games in between work for now. Like you, I'll give it a proper playthrough as well though, for now I just work on this mod on/off and that's pretty much it =)

@WelshGuyX
Copy link

@lyravega would it be possible to get a downloaded able version of you updates to this mod? I just want the money in station to be left alone, and not get spammed with station asking for operating capital

@lyravega
Copy link
Author

@lyravega would it be possible to get a downloaded able version of you updates to this mod? I just want the money in station to be left alone, and not get spammed with station asking for operating capital

https://github.com/lyravega/x4-warehouse-fleets

@GameBurrow
Copy link

GameBurrow commented Sep 2, 2024

Hey folks, any updates on this? Came here to suggest mutiple changes that are already mentioned here. Would be nice to see this merged into the main updated mod ;) (Mostly I need to toggle to disable messing with station account as I use other mod (Finance Hub: Transfers) to manage finances of all of my stations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants