We-the-People-civ4col-mod / Mod

This is the repository where the mod resides.
89 stars 37 forks source link

New diplomatic GUI #201

Open Nightinggale opened 5 years ago

Nightinggale commented 5 years ago

Problem: the diplo GUI is part of the exe and can't be modded much. This results in the following issues:

I propose

Make a new screen from scratch. This will once and for all remove the need to hack the far less than perfect exe solution. We can write the entire GUI in a new python file and it can be made to look the same and we can mod how it looks. For instance we can set the size according to screen resolution meaning it won't be tiny when used in modern resolutions.

BTS support

This has been on the BTS wishlist for years. If we make as much as possible in new files, half of making a BTS modcomp should be doable by copying those files. The rest should be copy paste in files where Colo and BTS have the same classes/functions.

There are skilled people working on BTS mods and we can get far if we work together when we have a common goal. This means if we plan BTS support from the start, then it could turn into a community task with people from multiple mods working together.

Data handling

We should add a new C++ class, which I will call conversation class here. It is intended to store all the data for an ongoing diplo screen. This way the python interface can be reduced to a display only while the actual workload is in C++, which is easier to debug. It also allows adding assert checks.

The conversation class should be added to the savegame, which will likely be needed for network sync issues. #46 has a proposal for a savegame layout, which is interesting and way easier to implement in a new class than converting existing classes. This means we can make a proof of concept without much extra work.

Network communication

In order to stay in sync in network games, some network communication is needed. I propose the approach where all computers have the conversation class and each change is transmitted to make all computers in sync. The reason is this:

The way to communicate is to transmit each item in the deal individually and we will have to rely on:

void CvDLLUtilityIFaceBase::sendPlayerAction(PlayerTypes ePlayer, PlayerActionTypes eAction, int iData1, int iData2, int iData3);
void CvPlayer::doAction(PlayerActionTypes eAction, int iData1, int iData2, int iData3);

This means we have an action and 3 ints. If we need more than 3 ints, what we will have to do is to send one action, store the result and then send another action. Network packages are handled FIFO according to my experience/tests. This means we have in theory unlimited number of ints to explain what we want to do, though the more packages we send, the more prone to network lag the screen will become.

In case multiple screens exist at the same time, we will likely have to set both players in the network package. One is ePlayer. The issue is the other player. If it is set as iData1, it will increase the number of packages by 50%. Instead we should set it in eAction. When receiving, we do:

PlayerTypes eOtherPlayer = static_cast<PlayerTypes>((eAction >> 24) & 0xFF);
eAction &= 0xFFFFFF;

Sending is then done through a dedicated function in the conversation class, which hides that it will actually store two numbers in the same int. This gives 24 bit unsigned int for player action types, which is 16.7 million. If you plan to use more than that, then you are doing something very wrong.


Note: ideally we should mod CvMessageControl to handle this, but that class in BTS only. For unknown reasons it's inside the exe in Colonization, meaning any approach to shared code will have to use vanilla CvMessageControl, hence why I propose hooking this into PlayerActionTypes.

orlanth commented 5 years ago

Didn’t DoaNE already redo that interface? If so maybe that could be re-used to save time. I’m surprised to hear it’s all in the exe, IIRC even Kailrics inventor modcomp (used for old 2071 mod) had modded it to add trading techs.

Nightinggale commented 5 years ago

It's not all in the exe. It's cooperation between the exe and DLL. The problem is that we apparently can add items to the list, but they are either included or not. Also adding an item will not exclude other items.

Example: A settlement has 3 different yields for sale. A wagon train (2 cargo slots) shows up and wants to buy stuff. The player can add all 3 kinds of yields despite the wagon train not having room for them because the exe generated the list when the window was opened and for each yield it can be added (if you don't add anything else). It doesn't check if all the yields added to the deal can fit in the wagon train at the same time. This seems unfixable while still using the exe.

It is possible to add techs in DLL/xml because there is no cap against adding too many techs at once. If we mod the code, it will be easier and use nicer code to add, but it's not impossible to add techs to vanilla. The problem with the exe is that often it's possible to get something working sort of in the direction you want it to work, but it's a compromise because there isn't modding freedom and it might not be possible to do it precisely like you want to do it.

Rather than keep on working around the limitations, we might as well go all out and make our own now that we have a growing list of bugs, which seems to be very difficult or impossible to fix while still using the exe approach.

Didn’t DoaNE already redo that interface? Looks like it has CvNativeYieldTrade.py. It looks like it's written exclusively for trading with AI natives, meaning no support for human vs human communication and not for talking to non-natives. It's not bad and will likely work just fine for the task it's designed to do, but I would prefer something more generic than that, something which can be used to replace the exe diplo system entirely and not just when trading with the natives.

orlanth commented 5 years ago

Ok :) if eventually redoing diplo screen, a cool option to add would be a treaty for Trade Access (or Embargo to cancel trade access). This could also work great with the tradescreens/europescreens becoming moddable: if it’s made possible for a Civ to control access to a tradescreen (such as King civs controlling access to their appropriate national tradescreen) modders could make some tradescreens where diplomacy is an important consideration for market access.

LibSpit commented 5 years ago

If a player could build a wonder like the market fair in M:C, they could have sole control of a trade location and allow access to others.

devolution79 commented 5 years ago

f1rpo claims to have found a possible work-around for the "bug in the EXE that prevents AI-to-human peace offers from being shown"

https://github.com/f1rpo/AdvCiv/commit/b9397691e4d045403bce7d920fa14b401c8c4448

Nightinggale commented 3 years ago

I looked through the ability to send network commands, which will be required if a new diplo screen is to be made. Since we can't code a new command from scratch, we need to hijack an existing one.

Candidates:

virtual void sendDoTask(int iCity, TaskTypes eTask, int iData1, int iData2, bool bOption, bool bAlt, bool bShift, bool bCtrl) = 0;
virtual void sendAdvancedStartAction(AdvancedStartActionTypes eAction, PlayerTypes ePlayer, int iX, int iY, int iData, bool bAdd) = 0;
virtual void sendUpdateCivics(CivicTypes* paeCivics) = 0;

Network packages work the same way as savegames in the sense that it's a byte stream. It doesn't matter if the exe is compiled with bool or int as either will transmit 4 bytes. In other words this is a question of how many ints each package can contain. Each package has an enum value telling what the package is doing and an int, which the exe will translate to a pointer to an object, like an instance of CvPlayer. Not including those, the amount of available data will be: sendDoTask: 6 ints sendAdvancedStartAction: 4 ints sendUpdateCivics: 1 int for each civic option Skipped less useful, but possible options like sendPlayerAction because with just 3 ints, it's worse.

sendUpdateCivics holds the potential for the most data as we can keep adding categories if needed. However it might be really hard to receive this and it quickly gets messy. The question is then if we should go with 6 ints for a city or 4 ints for a player.

Player has the obvious advantage of players always being available while a city needs a city to work on. However linking to a city could allow much more network data if we are a bit creative.

If we use sendAdvancedStartAction to init the window in sync, then the synced window can create an instance of CvCityAI. The ID from this one can then be used for sendDoTask. This city then contains all the negotiation data, either as a pointer to an instance of a different class or a union since normal city data won't be needed. Most likely a pointer to a python exposed dedicated class will be the cleanest implementation. What is added to the negotiation can then be set with TaskTypes. We can then decide that by default it is proposed to be an item added to the left player, but if the right player adds it, we add NUM_TASK_TYPES. That way eTask can contain both the type to add and which side using just a single variable. This leaves a total of 6 ints to detail the items we want to add/modify/remove in the negotiations.

The negotiation data should be synced, not just between the negotiating players, but all computers because that way once both players send an accept over the network, all computers can in sync activate the deal.

The negotiation needs to be saved or the game will desync if a player joins while two other players are negotiating.

Still no idea if we will do this, but I started thinking about this again yesterday and wondered about how precisely we can make a custom diplo screen work in network games.

We should use dedicated files as much as possible for this as that will make it easier to make a standard, which can be copied into other mods. There is an interest for this in the BTS community, but nobody there have started anything. We could make it a joined effort and for that purpose sendAdvancedStartAction and sendDoTask are good candidates because they are identical in BTS and Colonization. That makes it easier to write code, which works for both despite the fact that the BTS DLL has differences regarding network traffic.