Skip to content
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

bIsPuff and damagesource for puffs #2591

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jekyllgrim
Copy link
Contributor

@jekyllgrim jekyllgrim commented Jun 3, 2024

Actors that were spawned by P_SpawnPuff will receive the new bISPUFF flag. This also adds a damagesource field to Actor which stores who fired the puff (similar to PUFFGETSOWNER + target, but unconditionally).

P.S. I don't know why but lline 708 kept being marked as changed for me when I added DamageSource to actor.zs. I'm guessing it has something to do with the fact that the comment on line 708 contains an invalid character.

@RicardoLuis0
Copy link
Collaborator

shouldn't bISPUFF and damagesource be marked as readonly?

@jekyllgrim
Copy link
Contributor Author

jekyllgrim commented Jun 3, 2024

shouldn't bISPUFF and damagesource be marked as readonly?

Since there's no behavior tied to them, right now there's no technical need to do that. But if something gets attached to them in the future, it might be a good idea. Changing them dynamically in ZScript doesn't do anything, so there's no reason not to make them readonly either.

@Boondorl
Copy link
Contributor

Boondorl commented Jun 8, 2024

Since there's no behavior tied to them, right now there's no technical need to do that. But if something gets attached to them in the future, it might be a good idea.

Usually it's best to set it early as, once it's freely modifiable, you can't change that without causing potential breakage. It's simpler to go from readonly -> mutable than it is to go from mutable -> readonly. Though frankly I'd argue knowing if something is a puff and where it came from isn't impactful enough to require that kind of guarding. Generally it's only things that have important side effects (e.g. NOBLOCKMAP) that need to be marked immutable so they can be handled through the proper methods.

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.

3 participants