Skip to content

Conversation

@is-this-c
Copy link
Contributor

@is-this-c is-this-c commented Sep 23, 2025

  • Changes derived from Alpine Faction's source code
    • Improves compatibility with Alpine Faction network protocol
      • Flags::allow_fb_mesh
      • Flags::allow_no_ss
      • Flags::no_player_collide
      • Flags::allow_no_mf
      • Flags::click_limit
      • af_damage_notify_packet
    • In Alpine Faction levels
      • Handles AlpineLevelProps::legacy_cyclic_timers
      • Allows triggers to activate events in multiplayer
      • Fixes events that broke if their delay parameter was set
      • Supports loading Alpine Faction's $Lightmap Clamp Floor, and $Lightmap Clamp Ceiling, $Crater Texture PPM, and $Mesh Replacementfrom {map_name}_info.tbl
      • Supports dynamic lights
      • Makes particle emitters placed in levels respect their active distance parameter
  • Prevents spawning in maps if there are unsupported event classes
  • New hit_sounds, weapon_screen_shake, full_bright_entities, and remote_server_flags commands

I did not bother cleaning up af_options.cpp so it is just a copy paste.

Resolves #182
Resolves #301

@is-this-c is-this-c marked this pull request as draft October 16, 2025 12:55
@is-this-c is-this-c requested a review from rafalh October 17, 2025 14:10
@is-this-c is-this-c marked this pull request as ready for review October 17, 2025 14:41
Copy link
Owner

@rafalh rafalh left a comment

Choose a reason for hiding this comment

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

This PR is getting out of hands. It's really hard to review 1000+ changes (125 commits) multiple times. It takes me a lot of time and I'm still not sure if I didn't skip something. Can you split it into smaller PRs instead of making one big PR with everything? You add new stuff with every revision and fix unrelated stuff like adding const everywhere. If you must add it, okay, but not make huge PRs like this which changes 10 things at the same time. And all those commits have bad titles like "Update". Should I squash them and be left with one big commit in the git history? Every unrelated feature should have it's own commit/PR. Big PR for adding all Alpine stuff is a bad idea.
Small PR would also make it easier for me to find time to review it...

const bool ir_scanner = (params.flags & MRF_SCANNER_1) != 0;
if (!ir_scanner) {
rf::Vector3 ambient_light{0.f, 0.f, 0.f};
light_get_ambient(&ambient_light.x, &ambient_light.y, &ambient_light.z);
Copy link
Owner

Choose a reason for hiding this comment

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

why it was moved? I can see in RF code that those builtin lights are used for characters too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is in render_v3d_vif which is for e.g. item pickups. Characters are lighted in render_character_vif.

const bool ir_scanner = (params.flags & MRF_SCANNER_1) != 0;
if (!ir_scanner) {
const bool allow_full_bright = !(get_remote_server_info()
&& !get_remote_server_info().value().allow_full_bright_entities);
Copy link
Owner

Choose a reason for hiding this comment

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

there should be no negation or it should be "deny", not "allow", because currently it makes no sense

Copy link
Contributor Author

@is-this-c is-this-c Oct 27, 2025

Choose a reason for hiding this comment

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

const bool allow_full_bright = !(get_remote_server_info()
    && !get_remote_server_info().value().allow_full_bright_entities);

seems clearer in expressions e.g. if (config.full_bright && allow_full_bright) instead of if (config.full_bright && !no_full_bright)

const bool no_full_bright = get_remote_server_info()
                && !get_remote_server_info().value().allow_full_bright_entities;

AlpineLevelInfoConfig g_af_level_info_config;

// trim leading and trailing whitespace
std::string trim(const std::string& str, bool remove_quotes = false)
Copy link
Owner

Choose a reason for hiding this comment

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

why this file recreates entire parsing module for no reason introducing bugs like not handling escape character in strings? You just copied it from Alpine right? Looks like AI slop :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't have the energy to go through it since it was an isolated file.

struct DashLevelProps
{
// backward compatible defaults
struct AlpineLevelProps {
Copy link
Owner

Choose a reason for hiding this comment

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

of course Goober had to copy my way of handling that, but of course he didn't try to handle my chunk, right? I wonder why he didn't make new cyclic timers depend on rfl version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alpine Faction handles both chunks. legacy_cyclic_timers does depend upon RFL version. See event_type_forwards_messages_patch.

};

struct AlpineLevelInfoConfig {
std::unordered_map<std::string, std::unordered_map<AlpineLevelInfoId, LevelInfoValue>> level_options{};
Copy link
Owner

Choose a reason for hiding this comment

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

what is this, some kind of cache that you never clean? another AI slop?

Copy link
Contributor Author

@is-this-c is-this-c Oct 27, 2025

Choose a reason for hiding this comment

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

Goober agreed it should be for the loaded level only. I've changed it.

[](rf::Object* objp, const char* name, rf::VMeshType type) {
rf::VMesh* mesh = obj_create_mesh_hook.call_target(objp, name, type);
[] (rf::Object* const objp, const char* name, const rf::VMeshType type) {
const std::input_iterator auto level = g_af_level_info_config
Copy link
Owner

Choose a reason for hiding this comment

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

level? that's a stupid name. Better mesh_replacements or replacements, because i'ts clearly not a level...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I missed this somehow.

int,
rf::Particle**,
rf::ParticleEmitter*
)> particle_create_level_emitter_hook{
Copy link
Owner

Choose a reason for hiding this comment

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

this name is confusing, it's pretty much a patch for ParticleEmitter::spawn method, but the name suggests it's some kind of constructor for particle emitter itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is called particle_create? What about particle_spawn_level_emitter_hook?

float current_state_time;
bool active;
int field_144;
int active_distance;;
Copy link
Owner

Choose a reason for hiding this comment

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

Double colon. Also is there a way to set this field in editor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

&& !get_remote_server_info().value().allow_full_bright_entities);
if (g_game_config.full_bright_entities && allow_full_bright) {
// For Direct3d 11, we set in `gr_d3d11_mesh.cpp`.
rf::Vector3& ambient_light = addr_as_ref<rf::Vector3>(0x01C3D548);
Copy link
Owner

Choose a reason for hiding this comment

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

it's ambient_color, not to be confused with 005A38D4. I didn't check in PS2 code, but that's how it's named in my PC data

Copy link
Contributor Author

@is-this-c is-this-c Oct 27, 2025

Choose a reason for hiding this comment

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

From IDA:

  ambient_color.x = g_ambient_light.x * 255.0;
  ambient_color.y = g_ambient_light.y * 255.0;
  ambient_color.z = g_ambient_light.z * 255.0;

Is it not the same?

// RF does not allow triggers to activate events in multiplayer.
// In AF levels, we allow it, unless it is an event that would be
// problematic in multiplayer.
const rf::EventType event_type = static_cast<rf::EventType>(event->event_type);
Copy link
Owner

Choose a reason for hiding this comment

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

(opinion) great idea, enable all events without any synchronization of the game state between server and client, especially late client. It probably introduces some quirks and even bugs, but who cares

@is-this-c
Copy link
Contributor Author

This PR is getting out of hands. It's really hard to review 1000+ changes (125 commits) multiple times. It takes me a lot of time and I'm still not sure if I didn't skip something. Can you split it into smaller PRs instead of making one big PR with everything?

Good point. But seems too late...

@is-this-c is-this-c changed the title Alpine compat Alpine 1.1 compat Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: kill beep/reward Disable camera shake command

3 participants