My thoughts, questions and wishes

Discuss API design here.
User avatar
Ayuto
Project Leader
Posts: 2047
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: My thoughts, questions and wishes

Postby Ayuto » Fri Dec 27, 2019 8:34 pm

1. I would use Satoon's suggestion. The iterator method is already meant as a method that should be implemented by the programmer if he would like to have its own implementation.

2. Which place do you suggest?

3. Just added a wrapper property to the Player class. With the next release, you can simply do the following.

Syntax: Select all

from players.entity import Player

player = Player(1)
print(player.net_info.time_connected)


7. Yes, what is wrong with get_interface? Do you have any problems?

8. We are accessing an external site, because it's the easiest way. Of course, the server also knows his public IP address, but AFAIK there is no easy or reliable way to retrieve it.

9. We only add modules/packages to our site-packages if Source.Python requires them.

10. I would rather add a generic API for worker threads. E. g. this one:

11. Yes! It might requires you to spend some time to get started with it, but in the end you will benefit from that, because the abstraction layer doesn't force the server owner to use a specific database system.

12. I'm against that. You only need a Player instance to access networked (or datamap) properties on a really low-level base (player.get_datamap_property_<type>, player.get_network_property_<type>). I don't see any benefit in adding functions that accept an index. In the background we still need a player pointer. So, we would end up doing something like this:

Syntax: Select all

def get_network_property(index, prop):
player = Player(index)
return player.get_network_property(prop)


14. You can hook virtual functions. Why do you think you can't hook virtual functions? Do you need an example?

15. You can either write your own functions that work with the mathlib instances. Or, if you think it's a useful addition for everyone, you can make a pull request to add additional features directly to the mathlib library.

16. A Color instance is really just a class that represents RGB values. Since a few games support RGB values in SayText2 messages, we decided to implement the __str__ method that converts the RGB values into a hex string that can be used in a SayText2 message. If you want to develop a cross-game compatible plugin, you should utilize the colors from the module messages.colors.saytext2.

20. I'm not fully sure if I understood that correctly: you would like to replace the Player class whose instances are passed to some listeners. E. g. you would like to pass an instance of MyPlayer to OnPlayerRunCommand listeners instead of instances of Player? If that's what you want, then the answer is No. It's not possible and we won't change that. Remember that multiple plugins might want to change the standard class. That would only lead to more difficulties.

Edit: This thread will probably get very confusing, so we might need to split it into smaller threads. Next time please create separate threads in the first place. Yes, even if that would mean to create 20 threads.
User avatar
InvisibleSoldiers
Member
Posts: 94
Joined: Fri Mar 15, 2019 6:08 am

Re: My thoughts, questions and wishes

Postby InvisibleSoldiers » Fri Dec 27, 2019 9:21 pm

Ayuto wrote:2. Which place do you suggest?

ButtonStatus to players.constants
get_button_combination_status to players.helpers

Ayuto wrote:3. Just added a wrapper property to the Player class. With the next release, you can simply do the following.

Syntax: Select all

from players.entity import Player

player = Player(1)
print(player.net_info.time_connected)


:smile:
Ayuto wrote:7. Yes, what is wrong with get_interface? Do you have any problems?

Can you give me an example? I tried everything but unsuccessfully. Linux.
Ayuto wrote:8. We are accessing an external site, because it's the easiest way. Of course, the server also knows his public IP address, but AFAIK there is no easy or reliable way to retrieve it.

Maybe need to view source code of status command's handler because ше cannot take the IP from the air, and if it can’t do it at all, it means connection problems with Steam that means, in principle, there shouldn't be any IP address.
Ayuto wrote:9. We only add modules/packages to our site-packages if Source.Python requires them.

It's a pity.
Ayuto wrote:10. I would rather add a generic API for worker threads. E. g. this one:

Can you explain in 2-3 words what exactly it does, and what is difference between simple threading.Thread(target=any_callback).start() and your version.
Ayuto wrote:11. Yes! It might requires you to spend some time to get started with it, but in the end you will benefit from that, because the abstraction layer doesn't force the server owner to use a specific database system.

I tried once. Scared that SP is using the legacy v1.0 version. :confused:. Do you recommend only Core or ORM?
Ayuto wrote:14. You can hook virtual functions. Why do you think you can't hook virtual functions? Do you need an example?

So it seemed to me. :frown:
User avatar
L'In20Cible
Project Leader
Posts: 1249
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: My thoughts, questions and wishes

Postby L'In20Cible » Fri Dec 27, 2019 9:44 pm

2. I'm against moving them. This would break all existing codes that currently import them from there (e.g. freeze_tag, avsd, etc.), and since they are primarily to use in conjunction of the buttons listeners, having them there really just regroup your imports.

4. You are looking for the m_flMaxspeed property:

Syntax: Select all

max_speed = player.get_network_property_float('m_flMaxspeed')
# Or
max_speed = player.get_key_value_float('maxspeed')

Adding a wrapper for it to PlayerMixin would be easy enough.

5. What you mean read-only?

InvisibleSoldiers wrote:I didn't know about it, but looking for SourceMod, there is a special function https://sm.alliedmods.net/new-api/conso ... tToConsole , but it's probably wrong to compare Pawn and Python because Python it's all-around language, and Pawn is sort of embedding language like Lua and we're getting closer to native Source SDK usage.
That function is a wrapper that internally call IEngineServer::ClientPrintF: https://github.com/alliedmodders/source ... 2592-L2606

You could call it directly using engines.server.engine_server.client_printf(index, message).

16. No. This would not really make sense, because Player is not a container. And it wouldn't be reliable, because it would need the player to be cached first. It would also cause conflicts with inheritances.

InvisibleSoldiers wrote:
Ayuto wrote:9. We only add modules/packages to our site-packages if Source.Python requires them.

It's a pity.

Just because Source.Python doesn't ship with a specific package, doesn't mean you can't ship your plugin with it. For example, you could simply add it to your plugin's directory and import it from your_plugin.the_package.
User avatar
InvisibleSoldiers
Member
Posts: 94
Joined: Fri Mar 15, 2019 6:08 am

Re: My thoughts, questions and wishes

Postby InvisibleSoldiers » Sat Dec 28, 2019 11:51 am

L'In20Cible wrote:2. I'm against moving them. This would break all existing codes that currently import them from there and since they are primarily to use in conjunction of the buttons listeners, having them there really just regroup your imports.

Sooner or later you'll have to break existing codes for the best.
L'In20Cible wrote:4. You are looking for the m_flMaxspeed property:

Syntax: Select all

max_speed = player.get_network_property_float('m_flMaxspeed')
# Or
max_speed = player.get_key_value_float('maxspeed')


It is more globally then just m_flMaxspeed.
But having the properties in PlayerMixin is good.
  1. m_flMaxspeed
  2. player.pointer + 0xEF8 (player's friction)
  3. m_Local.m_nOldButtons

Syntax: Select all

weapon = player.get_active_weapon()
maxspeed = weapon_scripts[weapon.weapon_name].maximum_player_speed if weapon else 260


L'In20Cible wrote:5. What you mean read-only?

You said that sometimes need to copy a vector to be sure that it won't change later due to reference.
L'In20Cible wrote:You could call it directly using engines.server.engine_server.client_printf(index, message).

:smile:
User avatar
Ayuto
Project Leader
Posts: 2047
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: My thoughts, questions and wishes

Postby Ayuto » Sat Dec 28, 2019 2:38 pm

InvisibleSoldiers wrote:
Ayuto wrote:7. Yes, what is wrong with get_interface? Do you have any problems?

Can you give me an example? I tried everything but unsuccessfully. Linux.


Syntax: Select all

# Tested on Linux with CS:S
from core import get_interface

ptr = get_interface('bin/engine_srv.so', 'EngineTraceServer003')
print(ptr.address)



InvisibleSoldiers wrote:
Ayuto wrote:8. We are accessing an external site, because it's the easiest way. Of course, the server also knows his public IP address, but AFAIK there is no easy or reliable way to retrieve it.

Maybe need to view source code of status command's handler because ше cannot take the IP from the air, and if it can’t do it at all, it means connection problems with Steam that means, in principle, there shouldn't be any IP address.
Like I said, accessing an external site is the easiest way. IIRC, retrieving from the server requires signatures/offsets, which does not only mean more work, but also more maintainance. Moreover, I don't think a function called get_public_ip should return nothing if the connection to Steam is missing. The function does exactly what its name says. It returns the public IP. I think you are rather looking for a function called has_steam_connection. That's something we could add.

If you would like to implement your own get_public_ip function based on the status command, the easiest way would be using server_output:
http://wiki.sourcepython.com/developing ... ver_output

InvisibleSoldiers wrote:
Ayuto wrote:9. We only add modules/packages to our site-packages if Source.Python requires them.

It's a pity.

We could think about adding another directory (e. g. custom-site-packages) for site-packages that are required by plugins or custom libraries. That way we don't mix the site-packages that are shipped with SP. That sounds to me better than putting it into the plugins directory.

InvisibleSoldiers wrote:
Ayuto wrote:10. I would rather add a generic API for worker threads. E. g. this one:

Can you explain in 2-3 words what exactly it does, and what is difference between simple threading.Thread(target=any_callback).start() and your version.

You asked for threaded SQL. This little snippet allows you to add a result handler that is executed in the main thread and all jobs that are added to the worker thread are executed in the same order like they have been added. That is not guaranteed if you start a thread for every SQL statement.

I might have misunderstood your intention, so I will simply ask: why is it so hard to create a thread for your SQL statements? Does a one-liner really justify extending SP's functionality?


InvisibleSoldiers wrote:
Ayuto wrote:11. Yes! It might requires you to spend some time to get started with it, but in the end you will benefit from that, because the abstraction layer doesn't force the server owner to use a specific database system.

I tried once. Scared that SP is using the legacy v1.0 version. :confused:. Do you recommend only Core or ORM?

We usually only update them in two cases: When we update SP's Python version or if bugs are reported. By the time we added/updated it, 1.0.14 was the latest release. Use whatever you prefer. I don't have any recommendations.

InvisibleSoldiers wrote:
Ayuto wrote:14. You can hook virtual functions. Why do you think you can't hook virtual functions? Do you need an example?

So it seemed to me. :frown:

viewtopic.php?f=20&t=2145&p=13268&hilit=make_virtual_function#p13268

Though, there are a few more possibilities. It depends on your use case.
User avatar
L'In20Cible
Project Leader
Posts: 1249
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: My thoughts, questions and wishes

Postby L'In20Cible » Sat Dec 28, 2019 11:39 pm

InvisibleSoldiers wrote:Sooner or later you'll have to break existing codes for the best.

If unavoidable, yes. Your suggestion to move imports around is unfortunately not one of them I'm afraid.

InvisibleSoldiers wrote:[*]player.pointer + 0xEF8 (player's friction)

No need to hard-code the offset, just use:

Syntax: Select all

friction = player.get_network_property_float('localdata.m_flFriction')


InvisibleSoldiers wrote:You said that sometimes need to copy a vector to be sure that it won't change later due to reference.

If you plan to store a Vector/QAngle to use later then yes, you should always make a copy to ensure you own it. Otherwise, if you store a Vector that is owned by the engine and part of an entity's structure, it can be mutated at any time.

Ayuto wrote:Like I said, accessing an external site is the easiest way. IIRC, retrieving from the server requires signatures/offsets, which does not only mean more work, but also more maintainance. Moreover, I don't think a function called get_public_ip should return nothing if the connection to Steam is missing. The function does exactly what its name says. It returns the public IP. I think you are rather looking for a function called has_steam_connection. That's something we could add.

If you would like to implement your own get_public_ip function based on the status command, the easiest way would be using server_output:
http://wiki.sourcepython.com/developing ... ver_output

Actually, getting the output of the status command could be achieved with the following:

Syntax: Select all

from cvars import ConVar

def get_local_ip():
hostip = ConVar('hostip').get_int()
return (
f'{(hostip >> 24) & 0x000000FF}.{(hostip >> 16) & 0x000000FF}.' +
f'{(hostip >> 8) & 0x000000FF}.{hostip & 0x000000FF}:' +
f'{ConVar("hostport").get_int()}'
)


But that is the local ip, not the public one. My proposal would be to keep the current implementation, and simply store it once on a constant so plugins don't have to do the request themselves. Not that it matter, since they can always store it once as well. EDIT: Just saw the output on the wiki containing the public IP, that seems to be specific to CS:GO as OB games are not including it.

Ayuto wrote:We could think about adding another directory (e. g. custom-site-packages) for site-packages that are required by plugins or custom libraries. That way we don't mix the site-packages that are shipped with SP. That sounds to me better than putting it into the plugins directory.

Yes, that could be a good idea. Though, the problem I can see with that is, if a plugin ships with a specific version of a package that is incompatible with another it could cause conflicts between plugins. Not that I think we should care.

Return to “API Design”

Who is online

Users browsing this forum: No registered users and 15 guests