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

Feature: orderlist Importing / Exporting. #679

Open
wants to merge 15 commits into
base: jgrpp
Choose a base branch
from

Conversation

lucaFiorini
Copy link

No description provided.

@lucaFiorini
Copy link
Author

the system is now fully operational, imporing and exporting should work.

}

//delete all orders before setting the new orders
DoCommandP(v->tile, v->index, v->GetNumOrders(), CMD_DELETE_ORDER | CMD_MSG(STR_ERROR_CAN_T_DELETE_THIS_ORDER));
Copy link
Owner

@JGRennison JGRennison Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: Actually, never mind that comment

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command scope should really be moved one level upwards because these commands are liable to create problems in multiplayer. Really importing should be atomic with respect to the command scope.

@@ -672,6 +793,8 @@ struct SaveLoadWindow : public Window {
break;

case WID_SL_DRIVES_DIRECTORIES_LIST: { // Click the listbox
printf("test");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things like these can probably be taken out

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops


new_order.AllocExtraInfo();

if (extraJson.contains("cargo-type-flags") && extraJson["cargo-type-flags"].is_array())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be better to stick to the OpenTTD code style (braces, etc) where practical.

auto& headJson = json["head"];
for (unsigned int i = 0; auto &SD : SD_data) {

headJson["scheduled-dispatch"][i++] = nlohmann::json::parse(SD.ToJSONString());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serialising a JSON string and then immediately parsing it suggests that a string is not really the right return type?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to avoid having to include the json library in the headers, but I guess you are right.
Should it be changed to just use a json object then?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would likely be better, I agree that including the JSON library everywhere is not ideal, but I can resolve that later on

@@ -1628,7 +1632,7 @@ struct OrdersWindow : public GeneralVehicleWindow {

bool InsertNewOrder(uint64_t order_pack)
{
return DoCommandPEx(this->vehicle->tile, this->vehicle->index, this->OrderGetSel(), order_pack, CMD_INSERT_ORDER | CMD_MSG(STR_ERROR_CAN_T_INSERT_NEW_ORDER), nullptr, nullptr, 0);
return DoCommandPEx(this->vehicle->tile, this->vehicle->index, 0, order_pack, CMD_INSERT_ORDER | CMD_MSG(STR_ERROR_CAN_T_INSERT_NEW_ORDER), nullptr, nullptr, 0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this? Normal order inserts still need to work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this was just an oversight fixing now

if (extraJson.contains("dispatch-index"))
extraJson["dispatch-index"].get_to(new_order.extra->dispatch_index);

if (extraJson.contains("xdata"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These bypass all the order flag and value validation, so likely some changes are needed here. Something for me to look into later probably.


#include "table/strings.h"

#include "3rdparty/robin_hood/robin_hood.h"

#include <3rdparty/nlohmann/json.hpp>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include is duplicated, and in general system header syntax should not be used for local includes.

@@ -367,9 +474,9 @@ struct SaveLoadWindow : public Window {
this->querystrings[WID_SL_FILTER] = &this->filter_editbox;
this->filter_editbox.cancel_button = QueryString::ACTION_CLEAR;

/* pause is only used in single-player, non-editor mode, non-menu mode. It
/* pause is only used in single-player, non-editor mode, non-menu mode, when not operationg on orderlists. It
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a lot of spelling errors like this

ds.UpdateScheduledDispatch(nullptr);
SetTimetableWindowsDirty(v, STWDF_SCHEDULED_DISPATCH);
if (scheduleJson == nullptr) {
if (p2 == 0) return CMD_ERROR;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check shouldn't be within DC_EXEC because text and exec runs should return the same command result (except where flagged otherwise).

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

Successfully merging this pull request may close these issues.

2 participants