Skip to content

Commit

Permalink
Update container OnRemove, CanRemove program parameters (#647)
Browse files Browse the repository at this point in the history
* Implementation
- Add `amount` to CanRemove script
- Add `split_from` to OnRemove script

* Tests

* Add docs and core-changes

* Fix tests... maybe?

* Restore testing for client events in container event tests
  • Loading branch information
KevinEady committed May 16, 2024
1 parent 9ebf0a0 commit d916f86
Show file tree
Hide file tree
Showing 16 changed files with 340 additions and 29 deletions.
10 changes: 9 additions & 1 deletion docs/docs.polserver.com/pol100/corechanges.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@
<ESCRIPT>
<header>
<topic>Latest Core Changes</topic>
<datemodified>05-15-2024</datemodified>
<datemodified>05-16-2024</datemodified>
</header>
<version name="POL100.2.0">
<entry>
<date>05-16-2024</date>
<author>Kevin:</author>
<change type="Fixed">When removing a partial stack from a container, the `item` parameter in the container's<br/>
OnRemoveScript will now be the partial stack itself (instead of the pre-split stack).</change>
<change type="Added">Parameter `split_from` to container OnRemoveScript</change>
<change type="Added">Parameter `amount` to container CanRemoveScript</change>
</entry>
<entry>
<date>05-15-2024</date>
<author>Kevin:</author>
Expand Down
7 changes: 5 additions & 2 deletions docs/docs.polserver.com/pol100/scripttypes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,12 @@ const INSERT_INCREASE_STACK := 2;</example>


<scripttype name="CanRemoveScript">
<prototype>program canremovescript(character, container, item, movetype)</prototype>
<prototype>program canremovescript(character, container, item, movetype, amount)</prototype>
<parameter name="character" value="Character Ref, or uninitialized, see below" />
<parameter name="container" value="Container Ref" />
<parameter name="item" value="Item Ref" />
<parameter name="movetype" value="Integer" />
<parameter name="amount" value="Integer" />
<return>If 1, item is allowed to be removed from the container, else it is not removed.</return>
<schedtype>Run To Completion</schedtype>
<def_prio>critical</def_prio>
Expand Down Expand Up @@ -400,12 +401,13 @@ const INSERT_INCREASE_STACK := 2;</example>


<scripttype name="OnRemoveScript">
<prototype>program onremovescript(character, container, item, item_amount, movetype)</prototype>
<prototype>program onremovescript(character, container, item, item_amount, movetype, split_from)</prototype>
<parameter name="character" value="Character Ref, or uninitialized, see below" />
<parameter name="container" value="Container Ref" />
<parameter name="item" value="Item Ref" />
<parameter name="item_amount" value="Integer" />
<parameter name="movetype" value="Integer" />
<parameter name="split_from" value="Item Ref, or uninitialized, see below" />
<return>return value ignored</return>
<schedtype>Run-To-Completion</schedtype>
<def_prio>critical</def_prio>
Expand All @@ -414,6 +416,7 @@ const INSERT_INCREASE_STACK := 2;</example>
<todefine>In the item's itemdesc.cfg entry, the 'OnRemoveScript' property defines the location of the on-remove script. This is in package format ':pkgname:scriptname', or if just 'scriptname' in the same package, or /scripts/control.</todefine>
<explain>This script allows side effects to be triggered as a result of the item removal.</explain>
<explain>IMPORTANT: the first parameter (character) MAY be passed as an uninitialized object if the item was not moved by character (i.e. by a script function or the core), so please check it before you use it.</explain>
<explain>split_from MAY be passed as an uninitialized object if the item was not created as a result of splitting an item's stack.</explain>
<example>
use uo;
program my_example_onremovescript(character, container, item, item_amount)
Expand Down
5 changes: 5 additions & 0 deletions pol-core/doc/core-changes.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
-- POL100.2.0 --
05-16-2024 Kevin:
Fixed: When removing a partial stack from a container, the `item` parameter in the container's
OnRemoveScript will now be the partial stack itself (instead of the pre-split stack).
Added: Parameter `split_from` to container OnRemoveScript
Added: Parameter `amount` to container CanRemoveScript
05-15-2024 Kevin:
Fixed: Client is now properly updated when adding a stack to a container which contains a matching
stack (a regression introduced in previous core change)
Expand Down
15 changes: 11 additions & 4 deletions pol-core/pol/containr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,8 @@ void UContainer::spill_contents( Multi::UMulti* multi )
}


void UContainer::on_remove( Mobile::Character* chr, Items::Item* item, MoveType move )
void UContainer::on_remove( Mobile::Character* chr, Items::Item* item, MoveType move,
Items::Item* split_from )
{
if ( !desc.on_remove_script.empty() )
{
Expand All @@ -790,7 +791,8 @@ void UContainer::on_remove( Mobile::Character* chr, Items::Item* item, MoveType
// Luth: 10/22/2008 - on_remove_script now called with all appropriate parameters
call_script( desc.on_remove_script, chrParam, new Module::EItemRefObjImp( this ),
new Module::EItemRefObjImp( item ), new Bscript::BLong( item->getamount() ),
new Bscript::BLong( move ) );
new Bscript::BLong( move ),
split_from ? split_from->make_ref() : Bscript::UninitObject::create() );
}
}

Expand Down Expand Up @@ -855,7 +857,8 @@ void UContainer::on_insert_add_item( Mobile::Character* mob, MoveType movetype,
}
}

bool UContainer::check_can_remove_script( Mobile::Character* chr, Items::Item* item, MoveType move )
bool UContainer::check_can_remove_script( Mobile::Character* chr, Items::Item* item, MoveType move,
u16 amount )
{
if ( !desc.can_remove_script.empty() )
{
Expand All @@ -864,8 +867,12 @@ bool UContainer::check_can_remove_script( Mobile::Character* chr, Items::Item* i
chrParam = chr->make_ref();
else
chrParam = Bscript::UninitObject::create();

if ( amount == 0 )
amount = item->getamount();

return call_script( desc.can_remove_script, chrParam, make_ref(), item->make_ref(),
new Bscript::BLong( move ) );
new Bscript::BLong( move ), new Bscript::BLong( amount ) );
}
else
{
Expand Down
5 changes: 3 additions & 2 deletions pol-core/pol/containr.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ class UContainer : public ULockable

// TODO: rework these too.
bool check_can_remove_script( Mobile::Character* chr, Items::Item* item,
MoveType move = MT_PLAYER );
void on_remove( Mobile::Character* chr, Items::Item* item, MoveType move = MT_PLAYER );
MoveType move = MT_PLAYER, u16 amount = 0 );
void on_remove( Mobile::Character* chr, Items::Item* item, MoveType move = MT_PLAYER,
Items::Item* split_from = nullptr );
virtual void printProperties( Clib::StreamWriter& sw ) const override;
virtual void readProperties( Clib::ConfigElem& elem ) override;
virtual Bscript::BObjectImp* get_script_member( const char* membername ) const override;
Expand Down
19 changes: 11 additions & 8 deletions pol-core/pol/getitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ void GottenItem::handle( Network::Client* client, PKTIN_07* msg )

if ( item->container )
{
if ( !item->container->check_can_remove_script( client->chr, item ) )
if ( !item->container->check_can_remove_script( client->chr, item,
UContainer::MoveType::MT_PLAYER, amount ) )
{
send_item_move_failure( client, MOVE_ITEM_FAILURE_CANNOT_PICK_THAT_UP );
return;
Expand Down Expand Up @@ -168,13 +169,6 @@ void GottenItem::handle( Network::Client* client, PKTIN_07* msg )
item->gotten_by( client->chr );
item->setposition( Pos4d( 0, 0, 0, item->realm() ) ); // don't let a boat carry it around

if ( orig_container != nullptr )
{
orig_container->on_remove( client->chr, item );
if ( item->orphan() )
return;
}

/* Check for moving part of a stack. Here are the possibilities:
1) Client specified more amount than was in the stack.
2) Client specified exactly what was in the stack.
Expand All @@ -194,6 +188,9 @@ void GottenItem::handle( Network::Client* client, PKTIN_07* msg )
new_item->setposition( orig_pos );
if ( orig_container != nullptr )
{
orig_container->on_remove( client->chr, item, UContainer::MoveType::MT_PLAYER, new_item );
if ( new_item->orphan() )
return;
// NOTE: we just removed 'item' from its container,
// so there's room for new_item.
if ( !orig_container->can_add_to_slot( oldSlot ) || !item->slot_index( oldSlot ) )
Expand All @@ -219,6 +216,12 @@ void GottenItem::handle( Network::Client* client, PKTIN_07* msg )
}
else
{
if ( orig_container )
{
orig_container->on_remove( client->chr, item );
if ( item->orphan() )
return;
}
item->set_decay_after( 60 );
}

Expand Down
8 changes: 8 additions & 0 deletions testsuite/pol/scripts/include/testutil.inc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ function ret_error(str)
return error{errortext:=str};
endfunction

function ret_error_not_equal( actual, expected, message )
if ( actual != expected )
return ret_error( $"{message}" );
endif

return 1;
endfunction

function createAccountWithChar(accname, psw)
var a:=FindAccount(accname);
if (!a)
Expand Down
215 changes: 215 additions & 0 deletions testsuite/pol/testpkgs/client/test_client_container_events.src
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
include "communication";
include "testutil";

use os;
use uo;
use boat;
use file;
use polsys;

var char;
var charX := 100;
var charY := 50;
var clientcon := getClientConnection();

program test_item_move()
var a := FindAccount( "testclient0" );
char := a.getcharacter( 1 );
if ( !char )
return ret_error( "Could not find char at slot 1" );
endif

// Move character somewhere nice.
var res := MoveObjectToLocation( char, charX, charY, 0 );
if ( !res )
return ret_error( $"Could not move character: ${res}" );
endif

return 1;
endprogram

exported function test_container_events()
var ret, container, item, remaining_item;
do
var evs, expected_caninsert_args, expected_oninsert_args, expected_canremove_args, expected_onremove_args;

container := CreateItemAtLocation( char.x, char.y, char.z, "container_events" );
if ( !container )
ret := ret_error( $"Could not create container: {container}" );
break;
endif

SetObjProperty( container, "#TestPid", GetPid() );

// ----------
// Create 750 coins on ground.
// ----------
item := CreateItemAtLocation( char.x, char.y, char.z, "goldcoin", amount := 750 );
if ( !item )
ret := ret_error( $"Could not create item: {item}" );
break;
endif

// ----------
// Lift and drop 500 coins from the ground into the container.
// ----------
if ( !( evs := lift_item( ret, item.serial, 500, {}, { EVT_LIFT_ITEM } ) ) )
break;
endif

if ( !( evs := drop_item( ret, item.serial, 0xFF, 0xFF, 0, container.serial, { "container_caninsert", "container_oninsert" }, { EVT_DROP_ITEM } ) ) )
break;
endif

expected_caninsert_args := { char, container, MOVETYPE_PLAYER, INSERT_ADD_ITEM, item /* adding_item */, uninit /* existing_stack */, uninit /* amount_to_add */ };
expected_oninsert_args := { char, container, MOVETYPE_PLAYER, INSERT_ADD_ITEM, item /* adding_item */, item /* existing_stack */, 500 };

if ( !( ret := ret_error_not_equal( evs.container_caninsert.args, expected_caninsert_args, $"evs.container_caninsert.args {evs.container_caninsert.args} != expected_caninsert_args {expected_caninsert_args}" ) ) )
break;
endif

if ( !( ret := ret_error_not_equal( evs.container_oninsert.args, expected_oninsert_args, $"evs.container_oninsert.args {evs.container_oninsert.args} != expected_oninsert_args {expected_oninsert_args}" ) ) )
break;
endif

// ----------
// Fail to lift 250 coins from the container, as cannot remove odd-amount items from the container because of the CanRemove script
// ----------
if ( !( evs := lift_item( ret, item.serial, 251, { "container_canremove", EVT_MOVE_ITEM_REJECTED }, {} ) ) )
break;
endif

expected_canremove_args := { char, container, item, MOVETYPE_PLAYER, 251 };

if ( !( ret := ret_error_not_equal( evs.container_canremove.args, expected_canremove_args, $"evs.container_canremove.args {evs.container_canremove.args} != expected_canremove_args {expected_canremove_args}" ) ) )
break;
endif

// ----------
// Lift the remaining 250 from the ground and drop onto the stack of 500 in the container.
// ----------
remaining_item := ListItemsNearLocationOfType( char.x, char.y, LIST_IGNORE_Z, 0, "goldcoin" )[1];

if ( !remaining_item )
ret := ret_error( "Could not find remaining gold coin on ground." );
break;
endif

if ( !( evs := lift_item( ret, remaining_item.serial, 250, {}, { EVT_LIFT_ITEM } ) ) )
break;
endif

if ( !( evs := drop_item( ret, remaining_item.serial, 0xFF, 0xFF, 0, item.serial, { "container_caninsert", "container_oninsert" }, { EVT_DROP_ITEM } ) ) )
break;
endif

expected_caninsert_args := { char, container, MOVETYPE_PLAYER, INSERT_INCREASE_STACK, remaining_item, item, 250 };
if ( !( ret := ret_error_not_equal( evs.container_caninsert.args, expected_caninsert_args, $"evs.container_caninsert.args {evs.container_caninsert.args} != expected_caninsert_args {expected_caninsert_args}" ) ) )
break;
endif

// `adding_item` is uninit, because the item is added between CanInsert
// and OnInsert, and is therefore stacked _into_ the existing stack in
// the container (ie. `item.amount == 750`). We still have the `250` as
// the `amount_to_add` parameter.
expected_oninsert_args := { char, container, MOVETYPE_PLAYER, INSERT_INCREASE_STACK, uninit /* adding_item */, item, 250 };
if ( !( ret := ret_error_not_equal( evs.container_oninsert.args, expected_oninsert_args, $"evs.container_oninsert.args {evs.container_oninsert.args} != expected_oninsert_args {expected_oninsert_args}" ) ) )
break;
endif

if ( !( ret := ret_error_not_equal( item.amount, 750, $"item.amount ({item.amount}) != 750" ) ) )
break;
endif

// ----------
// Lift 250 coins from the container and drop into container (as a new item)
// ----------
if ( !( evs := lift_item( ret, item.serial, 250, { "container_canremove", "container_onremove" }, { EVT_LIFT_ITEM } ) ) )
break;
endif

expected_canremove_args := { char, container, item, MOVETYPE_PLAYER, 250 };
if ( !( ret := ret_error_not_equal( evs.container_canremove.args, expected_canremove_args, $"evs.container_canremove.args {evs.container_canremove.args} != expected_canremove_args {expected_canremove_args}" ) ) )
break;
endif

remaining_item := EnumerateItemsInContainer( container )[1];
if ( !remaining_item )
ret := ret_error( "Could not find remaining gold coin on ground." );
break;
endif

expected_onremove_args := { char, container, item, 250, MOVETYPE_PLAYER, remaining_item /* split_from */ };
if ( !( ret := ret_error_not_equal( evs.container_onremove.args, expected_onremove_args, $"evs.container_onremove.args {evs.container_onremove.args} != expected_onremove_args {expected_onremove_args}" ) ) )
break;
endif

// Already tested caninsert/oninsert for new item above, so just checking for DROP_ITEM
if ( !( evs := drop_item( ret, item.serial, 1, 1, 0, container.serial, {}, { EVT_DROP_ITEM } ) ) )
break;
endif

ret := 1;
dowhile ( false );

if ( container )
DestroyItem( container );
endif

if ( item )
DestroyItem( item );
endif

if ( remaining_item )
DestroyItem( remaining_item );
endif

return ret;
endfunction

/**
* Helper functions
*/

function get_container_events( byref err, sequential_events, other_events )
var evs := struct{};
var sequential_event_index := 1;
var other_event_index := 1;

while ( evs.size() < sequential_events.size() + other_events.size() )
var events := ( sequential_event_index <= sequential_events.size() ? array{ sequential_events[sequential_event_index] } : array{} ) +
( other_event_index <= other_events.size() ? array{ other_events[other_event_index] } : array{} );

var ev;
while ( 1 )
ev := waitForClients( events );

if ( !ev )
return err := ev;
elseif ( ev.type == other_events[other_event_index] )
other_event_index++;
elseif ( ev.type == sequential_events[sequential_event_index] )
sequential_event_index++;
endif

evs[ev.type] := ev;
break;
endwhile
endwhile

return evs;
endfunction

function drop_item( byref err, serial, x, y, z, dropped_on_serial, sequential_events, other_events )
Clear_Event_Queue();
clientcon.sendevent( struct{ todo := "drop_item", arg := struct{ serial := serial, x := x, y := y, z := z, dropped_on_serial := dropped_on_serial }, id := 0 } );

return get_container_events( err, sequential_events, other_events );
endfunction

function lift_item( byref err, serial, amount, sequential_events, other_events )
Clear_Event_Queue();
clientcon.sendevent( struct{ todo := "lift_item", arg := struct{ serial := serial, amount := amount }, id := 0 } );

return get_container_events( err, sequential_events, other_events );
endfunction

0 comments on commit d916f86

Please sign in to comment.