Skip to content

New parameter to FindValueInArray to compare with number of cells #966

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

Closed
wants to merge 10 commits into from
Closed

Conversation

TheDS1337
Copy link
Contributor

/**
 * Returns the index for the first occurrence of the provided value. If the value
 * cannot be located, -1 will be returned.
 *
 * @param array				Array Handle.
 * @param item				Value to search for
 * @param block				Optionally which block to search in
 * @param cellsToCompare	Optionally the number of cells to compare, 0 stands for the rest of the cells (Example: if you have a blocksize of 4 cells, and you want to compare the last 3 cells then: block = 1 and cellsToCompare should be 3)
 * @return					Array index, or -1 on failure
 * @error					Invalid Handle or invalid block
 */
native int FindValueInArray(Handle array, any item, int block=0, int cellsToCompare=0);

The new parameter 'cellsToCompare' will allow us to compare a certain number of cells, starting from 'block', since FindValue only compares a single cell right now,

It is very useful when working with structures that have array as members. Here's an example with an array of 2:
`#include

#pragma newdecls required

enum struct SomeStructure
{
int firstMember;
int otherMembers[3];
}

ArrayList g_Array;

public void OnClientSayCommand_Post(int client, const char[] command, const char[] sArgs)
{
int pos = -1;

if( strcmp(sArgs, "array_create", false) == 0 )
{
	g_Array = new ArrayList(sizeof(SomeStructure));

	if( g_Array )
	{
		PrintToChat(client, "Created array g_Array[x][%d] with success!", sizeof(SomeStructure));
	}
	
}
else if( strcmp(sArgs, "array_push", false) == 0 )
{
	SomeStructure data;
	data.firstMember = 6;
	data.otherMembers[0] = 1;
	data.otherMembers[1] = 0;
	
	if( g_Array.PushArray(data) != -1 )
	{	
		pos = g_Array.Length - 1;

		PrintToChat(client, "Pushed some structure into the entry %d", pos);			
		PrintToChat(client, "Expected values: firstMember = %d, otherMembers[2] = {%d, %d}", data.firstMember, data.otherMembers[0], data.otherMembers[1]);	
	}
}
// Adding  this just because I want more than 1 entry....
else if( strcmp(sArgs, "array_push_dummy", false) == 0 )
{
	SomeStructure data;
	data.firstMember = GetRandomInt(-1337, 1337);
	data.otherMembers[0] = GetRandomInt(-1337, 1337);
	data.otherMembers[1] = GetRandomInt(-1337, 1337);
	
	if( g_Array.PushArray(data) != -1 )
	{
		pos = g_Array.Length - 1;
		
		PrintToChat(client, "Pushed some structure into the entry %d", pos);			
		PrintToChat(client, "Expected values: firstMember = %d, otherMembers[2] = {%d, %d}", data.firstMember, data.otherMembers[0], data.otherMembers[1]);			
	}
}
else if( strcmp(sArgs, "array_find", false) == 0 )
{
	// Search starting from 1th cell
	if( (pos = g_Array.FindValue(6)) != -1 )
	{
		PrintToChat(client, "Found 6 at the entry %d", pos);
	}
	
	// Search starting from 2nd cell
	if( (pos = g_Array.FindValue(1, 1)) != -1 )
	{
		PrintToChat(client, "Found 1 at the entry %d", pos);
	}
	
	// Search starting from 3rd cell
	if( (pos = g_Array.FindValue(0, 2)) != -1 )
	{
		PrintToChat(client, "Found 0 at the entry %d", pos);
	}		
	
	// Works alright, but what if we want to compare a bigger number of cells? say 32 > 2?

	// comparing Array[X].otherMembers[1] and Array[X].otherMembers[2] parts of the blocksize
	if( (pos = g_Array.FindValue(1, 1)) != -1 && (pos = g_Array.FindValue(0, 2)) != -1 )
	{
		PrintToChat(client, "Found otherMembers[2] = {0, 1} at the entry %d", pos);
	}
	
	// comparing Array[X].firstMember and Array[X].otherMembers[0] parts of the blocksize
	if( (pos = g_Array.FindValue(6)) != -1 && (pos = g_Array.FindValue(1, 1)) != -1 )
	{
		PrintToChat(client, "Found firstMember = 6, otherMembers[0] = 1 at the entry %d", pos);
	}
}

}`

Output from chat:
DS‎ : array_create
Created array g_Array[x][4] with success!

DS‎ : array_push_dummy
Pushed some structure into the entry 0
Expected values: firstMember = -365, otherMembers[2] = {337, -269}

DS‎ : array_push_dummy
Pushed some structure into the entry 1
Expected values: firstMember = 131, otherMembers[2] = {738, 954}

DS‎ : array_push_dummy
Pushed some structure into the entry 2
Expected values: firstMember = -1279, otherMembers[2] = {-610, 440}

DS‎ : array_push_dummy
Pushed some structure into the entry 3
Expected values: firstMember = 762, otherMembers[2] = {426, -323}

DS‎ : array_push
Pushed some structure into the entry 4
Expected values: firstMember = 6, otherMembers[2] = {1, 0}

DS‎ : array_find
Found 6 at the entry 4
Found 1 at the entry 4
Found 0 at the entry 4
Found otherMembers[2] = {0, 1} at the entry 4
Found firstMember = 6, otherMembers[0] = 1 at the entry 4

It works fine, yes. but, imagine having an array with size bigger than 2?

…e number of cells

The new parameter 'cellsToCompare' will allow us to compare a certain number of cells, starting from 'block', since FindValue only compares a single cell right now,

It is very useful when working with structures that have array as members. Here's an example with an array of 2:
@asherkin
Copy link
Member

asherkin commented Mar 15, 2019

The default behaviour here appears to be a breaking change - looks like it should be 1 by default.

That said, I'm not sold on the feature.

EDIT: memcmp also doesn't do what you want and would be reading out-of-bounds if it even compiled.

@TheDS1337
Copy link
Contributor Author

The default behaviour here appears to be a breaking change - looks like it should be 1 by default.

That said, I'm not sold on the feature.

EDIT: memcmp also doesn't do what you want and would be reading out-of-bounds if it even compiled.

You're right, It should be 1 by default. On the other thing about memcmp... stupid me used the variables instead of their addresses, is that why it would fail?

@MitchDizzle
Copy link

This is interesting, I was also thinking of a "FindCellInArray" which would look through every entry and specifically at a cell index, along with being able to sort by cell index also.

Copy link
Member

@KyleSanderson KyleSanderson left a comment

Choose a reason for hiding this comment

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

Solid start, can you throw the include changes in here too?

With this being said, I'm in the same boat as Asher: a trie seems more natural for this usage.

@TheDS1337
Copy link
Contributor Author

Just updated, should be pretty now

I just noticed that I forgot to get the physical address of 2nd param, since it can be an array and that's the main purpose here.
@Headline Headline added the Feature Request user requested feature label Apr 21, 2019
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

Could you please add a test suite for this to plugins/testsuite/, looking at the implementation here the memcmp issue still seems to be present and doing invalid access to the plugin memory, so I'm not convinced this has been exhaustively tested.

Given the complexity this is adding to a common use case, and the limitations this has it seems it'll likely need extending in the future, I think it might be best to add a new FindArrayInArray native that takes an array of bytes to search for along with an offset to start matching from.

FindValueInArray(arr, 42, 1, 2); would become FindArrayInArray(arr, { 42, 42 }, 2, 1);

But this design also allows more flexible combinations such as FindArrayInArray(arr, { 11, 42 }, 2, 1);

And if we specify an additional limit param and wrap access to the input array, the first example can be simplified to: FindArrayInArray(arr, { 42 }, 1, 1, 2);

native int FindArrayInArray(Handle array, any[] needle, int needleLength, int start = 0, int limit = 0); 

Where:

  • needle = the bytes to search.
  • start = the byte offset in each block to start searching.
  • limit = the number of bytes of the block to match, looping needle as needed, with the following special values:
    • -1 = the block size - start
    • 0 = needleLength
  • needleLength > limit would be an error.
  • limit > (block size - start) would be an error.
  • 0 < start <= block size would be an error.

What do you think? Also @Headline.


for (unsigned int i = 0; i < array->size(); i++)

size_t cellsToCompare = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Default value still needs changing here.

Copy link
Contributor Author

@TheDS1337 TheDS1337 Apr 23, 2019

Choose a reason for hiding this comment

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

I've just built the fork for windows, and did a test.. using this :

#include <sourcemod>

#pragma newdecls required

enum struct SomeBlock		// 3 cells, 12 bytes, different memory representations
{
	int i;
	float f;
	bool b;
}

#define INSTANCE_INT 1337
#define INSTANCE_FLOAT 1337.0
#define INSTANCE_BOOL true

// Almost duplicate to instance1, just to test that it'll fall on this one if I compared the entirety of instance1
#define INSTANCE2_INT INSTANCE_INT
#define INSTANCE2_FLOAT INSTANCE_FLOAT
#define INSTANCE2_BOOL false

#define INSTANCE3_INT 10
#define INSTANCE3_FLOAT 0.0
#define INSTANCE3_BOOL false

ArrayList g_ArrayOfArrays;

public void OnPluginStart()
{
	RegServerCmd("test_create_array", test_create_array);
	RegServerCmd("test_fill_array", test_fill_array);
	RegServerCmd("test_search_array1", test_search_array1);
}

public Action test_create_array(int argc)
{
	const int blockSize = sizeof(SomeBlock); 
	
	g_ArrayOfArrays = new ArrayList(blockSize)
	
	if( g_ArrayOfArrays )
	{
		PrintToServer("Created an array with a block size of: %d", blockSize);
	}
	
	return Plugin_Continue;
}

public Action test_fill_array(int argc)
{
	static SomeBlock instance; instance.i = INSTANCE_INT; instance.f = INSTANCE_FLOAT; instance.b = INSTANCE_BOOL;
	g_ArrayOfArrays.PushArray(instance);
	
	static SomeBlock instance2; instance2.i = INSTANCE2_INT; instance2.f = INSTANCE2_FLOAT; instance2.b = INSTANCE2_BOOL;
	g_ArrayOfArrays.PushArray(instance2);

	static SomeBlock instance3; instance3.i = INSTANCE3_INT; instance3.f = INSTANCE3_FLOAT; instance3.b = INSTANCE3_BOOL;
	g_ArrayOfArrays.PushArray(instance3);
	
	PrintToServer("Added 3 arrays to g_ArrayOfArrays");
	
	return Plugin_Continue;
}

public Action test_search_array1(int argc)
{
	SomeBlock instance;	int pos = -1;	

/////// Search for the first instance,
	instance.i = INSTANCE_INT; 
	instance.f = INSTANCE_FLOAT; 
	instance.b = false;	// modified from "true" to false
	
	PrintToServer("Search for first array using SomeBlock::i and SomeBlock::b");
	
	// The search should start from the very first block because we are comparing the first 2 members (2 cells in this case) only
	if( (pos = g_ArrayOfArrays.FindValue(instance.i, 0, 2)) != -1 )
	{		
		PrintToServer("*	Found it at the position: %d", pos);		// Expectation: pos = 0
	}
	else
	{
		PrintToServer("*	Could not find the array with this pattern.");
	}
	
/////// Search for the third instance
	instance.i = INSTANCE_INT; 
	instance.f = INSTANCE3_FLOAT; // Modified float this time too
	instance.b = INSTANCE3_BOOL;	// modified from "true" to false, so it should fall back on the 3rd item
	
	PrintToServer("Search for third array using SomeBlock::f and SomeBlock::b");
	
	// We do the exact same, except this time we start from the 2nd block
	if( (pos = g_ArrayOfArrays.FindValue(instance.i, 1, 2)) != -1 )
	{		
		PrintToServer("*	Found it at the position: %d", pos);		// Expectation: pos = 2
	}
	else
	{
		PrintToServer("*	Could not find the array with this pattern.");
	}
	
/////// Search for the second instance
	instance.i = INSTANCE2_INT; 
	instance.f = INSTANCE2_FLOAT; 
	instance.b = INSTANCE2_BOOL;	// modified from "true" to false
	
	PrintToServer("Search for second array using SomeBlock::i, SomeBlock::f and SomeBlock::b!");
	
	// We compare the entire structure
	if( (pos = g_ArrayOfArrays.FindValue(instance.i, 0, 0)) != -1 )
	{		
		PrintToServer("*	Found it at the position: %d", pos);		// Expectation: pos = 1
	}
	else
	{
		PrintToServer("*	Could not find the array with this pattern.");
	}
	
	return Plugin_Continue;
}

Output from console:

test_create_array
Created an array with a block size of: 3
test_fill_array
Added 3 arrays to g_ArrayOfArrays
test_search_array1
Search for first array using SomeBlock::i and SomeBlock::b
*       Found it at the position: 0
Search for third array using SomeBlock::f and SomeBlock::b
*       Could not find the array with this pattern.
Search for second array using SomeBlock::i, SomeBlock::f and SomeBlock::b!
*       Found it at the position: 0

It doesn't work, you're right... it could be the memcmp implementation I'm doing is wrong. or it could be that 'instant.i' passed instead of 'instant' in the FindArray functions, from a C perspective I guess its totally okay, but I dunno how SP really handles them, I expect the same.

@Headline
Copy link
Member

FindArrayInArray sounds like a much better option, in my opinion. Good call @asherkin

@KyleSanderson
Copy link
Member

@TheDS1337 so this needs to be rebased. There's still what feels like (readas: destroying) memory corruption / invalidity for what's being passed through memcmp. Is this still desired?

@TheDS1337
Copy link
Contributor Author

desired

Hello @KyleSanderson, I've been away from games and gaming for a time now, so I have become also inactive in the SM community. This implementation might be useful for some in the future, thought I am not sure if I'm ever gonna code again to use it.

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

Successfully merging this pull request may close these issues.

5 participants