Clone vehicle multiple times
Moderator: OpenTTD Developers
Clone vehicle multiple times
I just created my first patch, which allows the player to clone a vehicle multiple times (AFAIK this wasn't possible yet?). I'm not really new to programming in general, but I am to the OpenTTD source code, so I'm not really sure if I adhered to your standards properly.
Please check out the code and tell me what you think.
Some questions:
- How does the Window custom data container work? What I've done now is used a global scope variable to hold the number of clones. I did use a window data thingie, but I copied it from some other window's code and I'm not really sure how it works.
- What's the difference between window number and window classes?
- I added a string to the English language file. If this patch proves useful and later gets incorporated into the main or MiniIN branch, how does the translating to other languages occur?
Edit:
I forgot to add: use it by Ctrl+clicking on the 'Clone vehicles' button in a depot.
Please check out the code and tell me what you think.
Some questions:
- How does the Window custom data container work? What I've done now is used a global scope variable to hold the number of clones. I did use a window data thingie, but I copied it from some other window's code and I'm not really sure how it works.
- What's the difference between window number and window classes?
- I added a string to the English language file. If this patch proves useful and later gets incorporated into the main or MiniIN branch, how does the translating to other languages occur?
Edit:
I forgot to add: use it by Ctrl+clicking on the 'Clone vehicles' button in a depot.
- Attachments
-
- CloneAmount.png (34.43 KiB) Viewed 4747 times
-
- CloneAmount.diff
- (6.07 KiB) Downloaded 230 times
Last edited by nlmark on 05 Jan 2007 19:14, edited 1 time in total.
Looks great
I'd suggest you to change tabs between WC_CLONE_AMOUNT and = 0x5A into spaces, because we use tabs to align beginning of line, then we use spaces to align things in mid-line
should be
I'd suggest you to change tabs between WC_CLONE_AMOUNT and = 0x5A into spaces, because we use tabs to align beginning of line, then we use spaces to align things in mid-line
Code: Select all
WC_CLONE_AMOUNT = 0x5A
Code: Select all
WC_CLONE_AMOUNT = 0x5A
Thanks
I changed the spaces into tabs, and added new functionality: you can now distribute the first orders of the newly created clones evenly (so you don't get one big line of vehicles all moving to the same point).
See attached .diff and screenie.
I changed the spaces into tabs, and added new functionality: you can now distribute the first orders of the newly created clones evenly (so you don't get one big line of vehicles all moving to the same point).
See attached .diff and screenie.
- Attachments
-
- CloneAmount-v2.png (94.89 KiB) Viewed 4623 times
-
- CloneAmount-v2.diff
- (8.32 KiB) Downloaded 201 times
Thats so cool and Welcome to OpenTTD.
I think the new window could be incorporated into the existing depot window. The deport window could be an expandable window like the Station window for 'ratings'. When you click the 'Clone vehicles' button it could expand the window showing all the options.
Also I think there should also be a click able option for 'Clone vehicle and share orders' which is currently Ctrl-Click when cloning included with the other options.
edit, on a side note its common practice to include the revision number the Diff / patch was made on in the file name.
I think the new window could be incorporated into the existing depot window. The deport window could be an expandable window like the Station window for 'ratings'. When you click the 'Clone vehicles' button it could expand the window showing all the options.
Also I think there should also be a click able option for 'Clone vehicle and share orders' which is currently Ctrl-Click when cloning included with the other options.
edit, on a side note its common practice to include the revision number the Diff / patch was made on in the file name.
Isn't it possible to click the number and then type a new number in the input box, this is the way it works in the patches window.Ploes wrote:Would it not be possible to have the "number" of verhicals as a text field not the < and > arrows.
Otherwise if you want 10 buses and you have 1. You still have to click something 9 times.
Don't panic - My YouTube channel - Follow me on twitter (@XeryusTC) - Play Tribes: Ascend - Tired of Dropbox? Try SpiderOak (use this link and we both get 1GB extra space)
OpenTTD: manual #openttdcoop: blog | wiki | public server | NewGRF pack | DevZone
OpenTTD: manual #openttdcoop: blog | wiki | public server | NewGRF pack | DevZone
How about making a horizontal scrollbar to set the number?
Also another issue (that nobody mentioned). Say somebody decides to clone 50 vehicles, then this patch will use DoCommandP() 50 times. Each time it sends a package to the server. If the number is added to the arguments and the command itself loops, then DoCommandP() will only be called once. This will ensure that network usage will be constant nomatter how many clones are made. Creating too many commands at once can have a negative effect on performance as the server needs to send all of them to each client.
This can be done like:
CloneOneVehicle() would then be the new name of what is currently called CmdCloneVehicle(). You will need to modify all the places where cloning is called. Also this code would need cleanup as it's written quickly in a web browser... it looks horribly, but it's the idea in it that's important.
Also it goes without saying that testing will be needed as well, since I didn't even try to compile this
Also another issue (that nobody mentioned). Say somebody decides to clone 50 vehicles, then this patch will use DoCommandP() 50 times. Each time it sends a package to the server. If the number is added to the arguments and the command itself loops, then DoCommandP() will only be called once. This will ensure that network usage will be constant nomatter how many clones are made. Creating too many commands at once can have a negative effect on performance as the server needs to send all of them to each client.
This can be done like:
Code: Select all
/** Clone a vehicle. If it is a train, it will clone all the cars too
* @param tile tile of the depot where the cloned vehicle is build
* @param p1 the original vehicle's index
* @param p2 bit 0 = shared orders, else copied orders
* bit 1-8 = number of vehicles to clone
*/
int32 CmdCloneVehicle(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
{
uint16 counter = 0;
uint16 max = GB(p2, 8, 1);
bool share_order = HASBIT(p2, 0);
int32 temp_cost = 0, cost = 0;
for (; counter < max; counter++) {
temp_cost = CloneOneVehicle(tile, flags, p1, share_order);
if (CMD_ERROR(temp_cost) return temp_cost;
cost += temp_cost;
}
return cost;
}
Also it goes without saying that testing will be needed as well, since I didn't even try to compile this
I resolved the large number issue by creating a text input box that pops up as suggested (and v2 already contains Ctrl+click shortcuts for the arrows, which increases the number by 5). I fixed some bugs, but I still have one I haven't figured out yet: Ctrl+clicking 'Clone vehicles' thus popping up the extra window and then clicking the 'Clone vehicles' button again throws an assertion error
I'm still working on that.
@ Bjarni
I didn't know DoCommandP sends commands to the server (explains a lot though ), so besides updating the CmdCloneVehicle() function I'll need to create a function like CmdGotoToOrderNumber() which replaces the functionality in CcCloneVehicle() for distributing orders (they are not sent through a DoCommandP() wrapper function so it won't work in multiplayer)
Code: Select all
openttd: window.c:297: FindWindowZPosition: Assertion `wz < _last_z_window' failed.
@ Bjarni
I didn't know DoCommandP sends commands to the server (explains a lot though ), so besides updating the CmdCloneVehicle() function I'll need to create a function like CmdGotoToOrderNumber() which replaces the functionality in CcCloneVehicle() for distributing orders (they are not sent through a DoCommandP() wrapper function so it won't work in multiplayer)
Fixed the bug
- Attachments
-
- CloneAmount-v3.png (45.49 KiB) Viewed 4456 times
-
- CloneAmount-v3.diff
- Patch based on MiniIN rev7691
Known bugs:
- Distributing orders doesn't work in multiplayer (might cause desync?) - (9.73 KiB) Downloaded 208 times
Re: Clone vehicle multiple times
Hi
was this ever implemented? How do i install?
was this ever implemented? How do i install?
Re: Clone vehicle multiple times
You must either patch the source code for OpenTTD that was used to create this patch, or you must update the code to use it with the latest source code of OpenTTD. After that, you will need to compile the source code for OpenTTD yourself. Any savegames or scenario created with this compiled copy of OpenTTD will not longer be compatible with any other copy of OpenTTD.hughht5 wrote:was this ever implemented? How do i install?
Do you like drones, quadcopters & flying toys? Check out Drone Strike Force!
Base Music Sets: OpenMSX | Scott Joplin Anthology | Traditional Winter Holiday Music | Modern Motion Music
Other Projects: 2CC Trams | Modern Waypoints | Sprite Sandbox & NewGRF Releases | Ideabox | Town Names | Isle of Sodor Scenario | Random Sprite Repository
Misc Topics: My Screenshots | Forgotten NewGRFs | Unfinished Graphics Sets | Stats Shack | GarryG's Auz Sets
Base Music Sets: OpenMSX | Scott Joplin Anthology | Traditional Winter Holiday Music | Modern Motion Music
Other Projects: 2CC Trams | Modern Waypoints | Sprite Sandbox & NewGRF Releases | Ideabox | Town Names | Isle of Sodor Scenario | Random Sprite Repository
Misc Topics: My Screenshots | Forgotten NewGRFs | Unfinished Graphics Sets | Stats Shack | GarryG's Auz Sets
Who is online
Users browsing this forum: Ahrefs [Bot] and 4 guests